There are a some of things you can do to make this code better,
- The first is to use
ifstreams
and do file input/output the proper idiomatic way in a loop, don't use .eof()
to check for end of file in a loop condition (the answer linked in the comments is a good place to start if you want to know why),
- The second thing you want to check for validity of the file with a simple
if (!file)
its much cleaner IMO.
- The third thing is, when you have a local file handle like you do in your code, then you can just let it go out of scope and let the destructor cleanup the file and
close()
it, it's the C++ RAII way of doing things (notice that I have removed the open()
method to the constructor call (which does the same thing)
- Use
cerr
instead of cout
to report errors
- Use
char
instead of int
to represent characters
- Not a big change, but using
std::toupper
like advised in the other answer's comments is a good readable way to check for uppercase and lowercase values at the same time
#include <iostream>
#include <fstream>
#include <string>
#include <cctype>
using namespace std;
int main()
{
string boys, girls, name;
int rank;
char end = 'n';
while (std::toupper(end) == 'Y')
{
cout << "Enter a name to search";
cin >> name;
ifstream input{"Names2016"};
// change here
if (!input) {
cerr << "Failed to open file.\n";
}
while (input >> rank >> boys >> girls)
{
if (boys == name)
cout << name << " ranks " << rank << " among boys.\n";
if (girls == name)
cout << name << " ranks " << rank << " among girls.\n";
}
// change below, just let the file handle go out of scope and close
// input.close();
cout << "Would you like to search another name?\n"
<< "Enter Y for yes or N for no.\n";
cin >> end;
}
return 0;
}
But you can do better on the I/O if your file isn't guaranteed to change over different iterations (in which case you probably need to make sure that there is no race anyway, so I am assuming the file does not change much). Read in the file once and save that information to be used later
#include <iostream>
#include <fstream>
#include <string>
#include <cctype>
#include <unordered_map>
#include <vector>
using namespace std;
int main()
{
string boys_name, girls_name, name;
int rank;
char end = 'n';
ifstream input{"Names2016"};
if (!input) {
cerr << "Failed to open file" << endl;
}
// preprocess the information and store it in a map
// making a map from string to vector because it is unclear whether
// there is a 1-1 mapping from the name to the rank for each name
unordered_map<string, vector<int>> boys;
unordered_map<string, vector<int>> girls;
while (input >> rank >> boys_name >> girls_name) {
boys[boys_name].push_back(rank);
girls[girls_name].push_back(rank);
}
while (std::toupper(end) == 'Y')
{
cout << "Enter a name to search";
cin >> name;
// use the map to do the lookup, much faster than reading
// the entire file over and over again
}
return 0;
}