Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
  • Despite what you may have been taught, you should not get into the habit of using using namespace std using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you should just return 1 instead of calling exit(1). The latter is only necessary in other places from which you will need to terminate early.

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use std::eof() for detecting the end of the file. It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (i.e. bad bit)). Read this this for more information.

    Instead, read from the file and check for the bad bit using std::getline():

     while (std::getline(std::cin, line))
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If the user enters an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     std::cout << "Pick a number between 1 through 100. ";
     std::cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you should just return 1 instead of calling exit(1). The latter is only necessary in other places from which you will need to terminate early.

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use std::eof() for detecting the end of the file. It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (i.e. bad bit)). Read this for more information.

    Instead, read from the file and check for the bad bit using std::getline():

     while (std::getline(std::cin, line))
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If the user enters an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     std::cout << "Pick a number between 1 through 100. ";
     std::cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you should just return 1 instead of calling exit(1). The latter is only necessary in other places from which you will need to terminate early.

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use std::eof() for detecting the end of the file. It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (i.e. bad bit)). Read this for more information.

    Instead, read from the file and check for the bad bit using std::getline():

     while (std::getline(std::cin, line))
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If the user enters an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     std::cout << "Pick a number between 1 through 100. ";
     std::cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

File read code was incorrect
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238
  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you couldshould just return 1 instead of calling exit(1). The latter is only necessary in other places from which you will need to terminate early.

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use std::eof() for detecting the end of the file. It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (iei.e. bad bit)). Read this for more information.

    Instead, read from the file and check for the bad bit using std::getline():

     while (inputFile >>std::getline(std::cin, numberline))
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If they enterthe user enters an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     std::cout << "Pick a number between 1 through 100. ";
     std::cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you could just return 1 instead of calling exit().

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use eof() for detecting the end of the file. It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (ie bad bit)). Read this for more information.

    Instead, read from the file and check for the bad bit:

     while (inputFile >> number)
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If they enter an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     cout << "Pick a number between 1 through 100. ";
     cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you should just return 1 instead of calling exit(1). The latter is only necessary in other places from which you will need to terminate early.

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use std::eof() for detecting the end of the file. It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (i.e. bad bit)). Read this for more information.

    Instead, read from the file and check for the bad bit using std::getline():

     while (std::getline(std::cin, line))
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If the user enters an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     std::cout << "Pick a number between 1 through 100. ";
     std::cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

added 59 characters in body
Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341
  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you could just return 1 instead of calling exit().

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use eof() for detecting the end of the file. It will look for the badeof bit after reading the end of the stream (and will not even consider errors in reading (ie bad bit)). Read this for more information.

    Instead, read from the file and check for the bad bit:

     while (inputFile >> number)
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If they enter an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     cout << "Pick a number between 1 through 100. ";
     cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you could just return 1 instead of calling exit().

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use eof() for detecting the end of the file. It will look for the bad bit after reading the end of the stream. Read this for more information.

    Instead, read from the file and check for the bad bit:

     while (inputFile >> number)
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If they enter an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     cout << "Pick a number between 1 through 100. ";
     cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

  • Despite what you may have been taught, you should not get into the habit of using using namespace std, especially when you start writing larger programs.

  • Since you're in main(), you could just return 1 instead of calling exit().

  • You don't need to display an output if the file was successfully opened. You'll know it failed if the program terminates after asking for a filename.

  • Do not use eof() for detecting the end of the file. It will look for the eof bit after reading the end of the stream (and will not even consider errors in reading (ie bad bit)). Read this for more information.

    Instead, read from the file and check for the bad bit:

     while (inputFile >> number)
     {
     // ...
     }
    
  • You ask the user for input, but you only validate it once. If they enter an invalid value a second time, the program will continue, causing problems. All of that should be in a loop that will terminate once the user provides valid input.

     do
     {
     cout << "Pick a number between 1 through 100. ";
     cin >> value;
     } while (value < 1 || value > 100);
    
  • You don't need return 0 at the end of main(). Reaching the end already implies a successful termination, so the compiler will do this return for you.

added 240 characters in body
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238
Loading
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238
Loading
lang-cpp

AltStyle によって変換されたページ (->オリジナル) /