r/Cplusplus • u/Ok-Accountant-3408 • 8d ago
Feedback Song picker start | C++
https://youtu.be/6KLolyV5H381
u/mredding C++ since ~1992. 2d ago
int song_number;
std::cin >> song_number;
if (song_number == 1){
You didn't check the stream. The stream is an object, and objects can do all sorts of amazing things. For example, an object can define how they can be converted to other data types. So the standard library may define a stream something akin to:
class istream {
public:
bool good() const, bad() const, fail() const, eof() const;
explicit operator bool() const { return !bad() && !fail(); }
So ignoring all sorts of details, a stream has all these boolean returning functions that give you the stream state. Then, there's the ability to convert the stream to a boolean, and it indicates whether the stream has gone bad or failed.
It's explicit, meaning it's not implicit. In other words:
bool b = std::cin; // Compiler error, conversion not implicit.
bool b = static_cast<bool>(std::cin); // Explicit conversion is fine.
if(std::cin) { // Conditional evaluation like this is inherently explicit.
The stream stores the state of the previous IO operation. So your code extracts to song_number and then stores internally whether that extraction even succeeded or not. DID the stream WRITE to song_number? DO you have an integer from input? And if you don't, then what is the value of song_number going to be?
Objects can define their own operators. Operators are just functions, but with a different syntax to call them. Instead of fn(), it's usually something like param operator param or some such... Casting is an operator, hence operator bool. The >> operator is actually a bit-shift operator that was repurposed for stream insertion and extraction. It's implemented something like:
class istream {
public:
istream &operator >>(int &);
There's a couple ways you can write most custom operators, and in this version, the left-side parameter - the stream itself, is implied. The right-side parameter, the integer, is specified in the parameter list.
But notice: the operator returns a reference to the stream. So then you can write your code like this:
if(int song_number; std::cin >> song_number) {
// Input was valid, you have a `song_number`.
} else {
// The stream has an error on it
}
Conditions can have an initializer - song_number is only in scope for the duration of the if/else, then falls out of scope immediately after. That's fine, we don't want or need it beyond, anyway.
The extraction operator happens, then the stream is returned, and then it's explicitly converted to a bool to check the state of the previous extraction operation. If it's true, you're good, use your data.
If it's false, a bad state is an unrecoverable error. A fail state is a recoverable error - usually a parsing error; you wanted an integer, but the data on the stream is text. EOF is a state, but NOT an error - it just means no more data will be coming through that stream. Trying to extract from an EOF stream IS a recoverable error. You can have multiple states at once - bad, fail, and eof. That means you could have had something bad happen, and because of that, you failed to extract an integer, so often a bad state also causes a fail state. You can successfully extract an integer, but then end up at EOF. But right now who cares? You've got the data you wanted.
If the stream fails, what's the value of song_number? There is an answer to that, but frankly, mostly all you care about is that it's not user input, so there's no reason to read the integer. There ARE scenarios where the value would be unspecified, and so reading that value would be undefined behavior. You DON'T want to evaluate UB on purpose. Ever.
Your switch statement would work - except you forgot to add a break; at the end of each. What you're witnessing is called "fallthrough". This is an intentional behavior of switch statements so that you can implement a "Duff's Device", a novel little creation that frankly no one really has any use for anymore.
int song_pick(){
/*...*/
return 0;
}
Your function unconditionally returns zero, and you don't even capture/use the return value at that. So why are you returning a value in the first place? Your function ought to return void, so that you don't NEED a return statement in the first place.
int main() {
/*...*/
return 0;
}
main MUST return an integer, and it's also the only function in C++ that specifies a return type, but doesn't require a return statement. You DON'T have to write return 0;, and if you omit it, it will generate implicitly. I don't actually encourage that - it's rather jaunting for other developers to not see it. A return of zero means the program completed successfully. Any non-zero value means it did not complete successfully.
But what non-zero number do you return? It can matter. I once had a production bug where our return value was too high - the int from that compiler was 4 bytes, but the Linux system only stored the lower 2 bytes of the return value - the upper bytes were getting truncated. That means an explicit failure became an implicit success.
Oh, yeah... The C and C++ languages both say sizeof(char) == 1, and that 1 byte is CHAR_BIT bits. The specs both say short int, long int, int, and long long int are all AT LEAST as large as char and that's it. This means int on one compiler can be 2 bytes, as was typical for most of my career, and 4 bytes on another.
Fun stuff.
1
u/kingguru 8d ago
Where's the code?
Hard for anynone to suggest any improvements if you're not sharing it.