Skip to main content
added 1321 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341
ifstream inputFile;
string fileName;
int value, x;

 
// Rahter do this:

Declare them as close to the point where you use them as possible (it makes reading the code easier). Also in C++ we sometimes (quite often) use the side affects of the constructor/destructor to do work. So you don't want those firing until you need them to fire.

std::cout << "What is the file name?\n"; // Don't forget the '\n'
std::string fileName;
std::getline(std::cin, fileName);

Declare them as close to the point where you use themThe same apples for eof() as possibleit does (it makes reading the code easier)fail(). Also in C++ we sometimes (quite often)Don't use the side affects of the constructor/destructor to do workit. So you don't want those firing until you need them to firePrefer bad().

while (!inputFile.eof())
// This will result in an infinite loop if the bad bit is set.

Also note that the stream operator operator>> returns a reference to the stream. Which (as stated above) when used in a boolean context will convert itself to value that is useful using bad()

 while(file >> value)
 {
     // Loop is entered every time the read works.
     // This is because `file >> value` returns a reference to a stream.
     // When a stream is used in a boolean context like while(<stream>)
     // weill test itself using `bad()`
 }
// Always use std::getline to read user input.
// Always do the read and test in the same condition.
// Always check for bad bit.

std::string line;
int         value;
while(std::getline(std::cin, line))
{
    // Successfully read a line.
    // If we did not enter the loop then we reached eof.
    
    // So now we want to check what the user entered was a number.
    // So convert the user input line into a stream and read a number
    // from it (or try).
    std::stringstream linestream(line);
    if (linestream  >> value)
    {
         // You have successfully read a user value.
         // This means the read was good.
         // If there was text on the input then this would have failed.
         if (value >=1 && value <= 100)
         {
             break;  // exit the loop
         }
    }

    // If we reach here the user either entered invalid data
    // Which caused the the `bad` bit in `linestream` to be set
    // or the value was out of range.
    // Either way we just tell him to retry.

    // Note: because we create a new linestream each attempt
    //       this effectively resets the `bad` bit. If you want
    //       mess around with the bit flags in the stream that 
    //       is fine but I usually find this easier.

    std::cout << "Hey... Whats up dumb addass you can't read? Number between 1 and 100. Try again:\n"

    // The outer while loop will the try to get another value.

}

if (std::cin.eof())
{
   // We failed to get a get a good value.
   // The loop above exited because std::getline() failed
   // This happens when we reach eof thus the value has
   // not been set correctly.


   return 1; // or exit(1) if appropriate.
}

std::cout << "The value was good: " << value << "\n";
ifstream inputFile;
string fileName;
int value, x;

 
// Rahter do this:

std::cout << "What is the file name?\n"; // Don't forget the '\n'
std::string fileName;
std::getline(std::cin, fileName);

Declare them as close to the point where you use them as possible (it makes reading the code easier). Also in C++ we sometimes (quite often) use the side affects of the constructor/destructor to do work. So you don't want those firing until you need them to fire.

// Always use std::getline to read user input.
// Always do the read and test in the same condition.
// Always check for bad bit.

std::string line;
int         value;
while(std::getline(std::cin, line))
{
    // Successfully read a line.
    // If we did not enter the loop then we reached eof.
    
    // So now we want to check what the user entered was a number.
    // So convert the user input line into a stream and read a number
    // from it (or try).
    std::stringstream linestream(line);
    if (linestream  >> value)
    {
         // You have successfully read a user value.
         // This means the read was good.
         // If there was text on the input then this would have failed.
         if (value >=1 && value <= 100)
         {
             break;  // exit the loop
         }
    }

    // If we reach here the user either entered invalid data
    // Which caused the the `bad` bit in `linestream` to be set
    // or the value was out of range.
    // Either way we just tell him to retry.

    // Note: because we create a new linestream each attempt
    //       this effectively resets the `bad` bit. If you want
    //       mess around with the bit flags in the stream that 
    //       is fine but I usually find this easier.

    std::cout << "Hey... Whats up dumb add you can't read? Number between 1 and 100. Try again:\n"

    // The outer while loop will the try to get another value.

}

if (std::cin.eof())
{
   // We failed to get a get a good value.
   // The loop above exited because std::getline() failed
   // This happens when we reach eof thus the value has
   // not been set correctly.


   return 1; // or exit(1) if appropriate.
}

std::cout << "The value was good: " << value << "\n";
ifstream inputFile;
string fileName;
int value, x;

Declare them as close to the point where you use them as possible (it makes reading the code easier). Also in C++ we sometimes (quite often) use the side affects of the constructor/destructor to do work. So you don't want those firing until you need them to fire.

std::cout << "What is the file name?\n"; // Don't forget the '\n'
std::string fileName;
std::getline(std::cin, fileName);

The same apples for eof() as it does fail(). Don't use it. Prefer bad().

while (!inputFile.eof())
// This will result in an infinite loop if the bad bit is set.

Also note that the stream operator operator>> returns a reference to the stream. Which (as stated above) when used in a boolean context will convert itself to value that is useful using bad()

 while(file >> value)
 {
     // Loop is entered every time the read works.
     // This is because `file >> value` returns a reference to a stream.
     // When a stream is used in a boolean context like while(<stream>)
     // weill test itself using `bad()`
 }
// Always use std::getline to read user input.
// Always do the read and test in the same condition.
// Always check for bad bit.

std::string line;
int         value;
while(std::getline(std::cin, line))
{
    // Successfully read a line.
    // If we did not enter the loop then we reached eof.
    
    // So now we want to check what the user entered was a number.
    // So convert the user input line into a stream and read a number
    // from it (or try).
    std::stringstream linestream(line);
    if (linestream  >> value)
    {
         // You have successfully read a user value.
         // This means the read was good.
         // If there was text on the input then this would have failed.
         if (value >=1 && value <= 100)
         {
             break;  // exit the loop
         }
    }

    // If we reach here the user either entered invalid data
    // Which caused the the `bad` bit in `linestream` to be set
    // or the value was out of range.
    // Either way we just tell him to retry.

    // Note: because we create a new linestream each attempt
    //       this effectively resets the `bad` bit. If you want
    //       mess around with the bit flags in the stream that 
    //       is fine but I usually find this easier.

    std::cout << "Hey... Whats up dumb ass you can't read? Number between 1 and 100. Try again:\n"

    // The outer while loop will the try to get another value.

}

if (std::cin.eof())
{
   // We failed to get a get a good value.
   // The loop above exited because std::getline() failed
   // This happens when we reach eof thus the value has
   // not been set correctly.


   return 1; // or exit(1) if appropriate.
}

std::cout << "The value was good: " << value << "\n";
added 1321 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

Don't declare all your variables at the top of the function.

ifstream inputFile;
string fileName;
int value, x;


// Rahter do this:

std::cout << "What is the file name?\n"; // Don't forget the '\n'
std::string fileName;
std::getline(std::cin, fileName);

When testing a stream state best to use bad rather than any of the other states (as this catches all failures). Also note that when a stream is used in a boolean context (ie in an if statements condition) it converts itself into a value compatible with the context using the bad() method as a test.

std::ifstream file(fileName.c_str());
if (!file)              // don't need to call any functions.
{                      // This calls `bad()` and converts
                       // the result into a value useful for a test
                       // true if it is open and readable or false otherwise
                       // so !file is only true if you can read the file.
     return 1;
}

Declare them as close to the point where you use them as possible (it makes reading the code easier). Also in C++ we sometimes (quite often) use the side affects of the constructor/destructor to do work. So you don't want those firing until you need them to fire.

Don't declare all your variables at the top of the function.

ifstream inputFile;
string fileName;
int value, x;


// Rahter do this:

std::cout << "What is the file name?\n"; // Don't forget the '\n'
std::string fileName;
std::getline(std::cin, fileName);

When testing a stream state best to use bad rather than any of the other states (as this catches all failures). Also note that when a stream is used in a boolean context (ie in an if statements condition) it converts itself into a value compatible with the context using the bad() method as a test.

std::ifstream file(fileName.c_str());
if (!file)              // don't need to call any functions.
{                      // This calls `bad()` and converts
                       // the result into a value useful for a test
                       // true if it is open and readable or false otherwise
                       // so !file is only true if you can read the file.
     return 1;
}

Declare them as close to the point where you use them as possible (it makes reading the code easier). Also in C++ we sometimes (quite often) use the side affects of the constructor/destructor to do work. So you don't want those firing until you need them to fire.

Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

Everything @Jamal said:

When reading user input prefer not mix getline() with operator>> as one reads and discards new line while the other does not and this can cause confusion as to where on the input stream you are.

Secondly user input (when done manually) is usually line based (ie the input buffer is not flushed until they hit return). So it is best to read a whole line at a time then validate it.

Remember scope:

int value;   // This is the value being used in the while condition.

// STUFF

do
{
    cout << "Pick a number between 1 through 100. ";

    int value;   // This is the value you are
                 // reading from user input and has nothing to 
                 // do with the variable value in the outer scope.

                 // It is usually a bad idea to shadow variables
                 // in the outer scope like this as it causes confusion.
                 // so turn your compiler warning up so that the compiler
                 // warns you about variable shadowing.

                 // Then tell your compiler to treat all warnings as
                 // errors (as all warnings are really logicall errors
                 // in your code).

    cin >> value;

  // This while
  // is testing the variable `value` that is defined in the outer
  // scope (which is not what you want).
} while (value < 1 || value > 100);

You must also check for invalid input.

int value;
std::cin >> value;

What happens if I type 'fred<enter>'?
Personally I have forgotten if the variable is even changed (its in the standard somewhere). But the stream has its bad bit set. This means any further attempt to read from the stream will result in nothing happening so you can't even do it in a loop.

int value = 0; // make sure it has an initial value so
               // the while() test is valid even if the
               // user enters junk.
do
{
    std::cout << "Enter a value\n";
    std::cin >> value;
}
while (value < 1 || value > 100);

If I type 'fred' then this will enter an infinite loop (even if the user enters 10 next time around). Because the bad bit has been set and nothing will be read from the stream while this bit is set (its set because you are trying to read an int and you got fred (which is not an int)).

###Conclusion:

// Always use std::getline to read user input.
// Always do the read and test in the same condition.
// Always check for bad bit.

std::string line;
int         value;
while(std::getline(std::cin, line))
{
    // Successfully read a line.
    // If we did not enter the loop then we reached eof.
    
    // So now we want to check what the user entered was a number.
    // So convert the user input line into a stream and read a number
    // from it (or try).
    std::stringstream linestream(line);
    if (linestream  >> value)
    {
         // You have successfully read a user value.
         // This means the read was good.
         // If there was text on the input then this would have failed.
         if (value >=1 && value <= 100)
         {
             break;  // exit the loop
         }
    }

    // If we reach here the user either entered invalid data
    // Which caused the the `bad` bit in `linestream` to be set
    // or the value was out of range.
    // Either way we just tell him to retry.

    // Note: because we create a new linestream each attempt
    //       this effectively resets the `bad` bit. If you want
    //       mess around with the bit flags in the stream that 
    //       is fine but I usually find this easier.

    std::cout << "Hey... Whats up dumb add you can't read? Number between 1 and 100. Try again:\n"

    // The outer while loop will the try to get another value.

}

if (std::cin.eof())
{
   // We failed to get a get a good value.
   // The loop above exited because std::getline() failed
   // This happens when we reach eof thus the value has
   // not been set correctly.


   return 1; // or exit(1) if appropriate.
}

std::cout << "The value was good: " << value << "\n";