Your getData()
function has some problems:
void getData(){
int count = 0;
while(!file.eof()){ // this is almost never the correct check
string names[count]; // you declare new VLA:s (non-standard) every iteration
int votes[count]; // -"-
cin >> names[count]; // and you put a value in it out of bounds.
cin >> votes[count]; // -"-
count ++;
} // both arrays are destroyed here
}
file.eof()
does not return true
until you've tried to read beyond the end of the file. If you've read the last value, it will not be set. Only when you try next time will it be set.
- The arrays you declare inside the
while
loop will be destroyed at the end of the loop. After the loop is finished, you have no arrays.
- When you declare an array of
count
elements, you can access those elements using 0
to count-1
inclusive. You access element count
which is out of bounds so your program has undefined behaviour.
- VLA:s (variable length arrays) does not exist in standard C++ (but does as an extension in some compilers). If you know exactly how many elements you need to store, you can use
std::array
instead, but in this case, use a std::vector
.
- It uses a global
file
variable (that doesn't even exist). Try to stay away from global variables if you can.
The records in your data file should be kept together instead of putting each column in a separate array. A simple placeholder for each record in your file could look like this:
struct record {
std::string name{};
int vote{};
};
With that, you only need one array (or std::vector
).
std::vector<record> records;
It'd also be good if one could extract one complete record
from a stream using the same >>
operator as you used for int
and std::string
. Like this:
record temp; // declare a variable using your own type, "record"
while(file >> temp) { // read records until no more can be read
records.push_back(temp) // add one record to records
}
A function to read one record from an istream
, like an ifstream
:
std::istream& operator>>(std::istream& is, record& r) {
// You may want to use getline here instead in case the names contain spaces.
return is >> r.name >> r.vote; // extract name and vote from is and return is
}
The function takes both parameters (is
and r
) by reference. That means that whatever is done to the parameters inside the function affects the variables that were used to call the function. file >> temp
results in a call to the above function where is
is a reference to file
and r
is a reference to temp
.
For openFile()
I'd suggest:
std::ifstream openFile(const std::string& fileName) { // return the ifstream by value
return std::ifstream{fileName};
}
Getting the filename from the user doesn't have anything to do with opening the file, so get the filename before calling the function. The above function lets you call openFile()
and get an ifstream
in return:
std::ifstream file = openFile(fileName);
You can now call getData()
using file
, but it needs to be able to receive it. Standard stream objects can't be copied (passed by value), but we don't need to. Just make getData()
receive a reference to the stream. I'd make it an istream
instead of an ifstream
to be able to read from any istream
decendant:
std::vector<record> getData(std::istream& is) {
// create a vector, read data from "is" and put it in vector and return vector when done
}
When all is pieced together, you could have a main()
looking something like this:
int main() {
std::vector<record> records;
std::cout << "Enter the name of the file to open: ";
// use getline since a filename may contain spaces
if(std::string fileName; std::getline(std::cin, fileName)) {
// if "file" is in a good state after openFile(), call getData()
if(std::ifstream file = openFile(fileName)) {
records = getData(file);
} // "file" is automatically closed when it goes out of scope
}
// print what you collected
for(const record& r : records) {
std::cout << r.name << "\t" << r.vote << "\n";
}
}
The above uses If Statements with Initializer which is a C++17 feature to help create a narrow scope for variables.