0

I have a computer science assignment which requires me to have a separate function just to open the file, and then another function which will then process the data in that file and then some others to do some operations with that data. Anyways, I'm having trouble in how to be able to let other functions use that opened file. References with '&' or'*' are confusing me and I'm unsure if I have to use one or not, of course, though I'm pretty sure I'll have to pass at least something to the next function. The main intent when dealing with the file is to open it(openFile) and then have another function(getData) to sort the data into two different arrays. One for the names, and one for the amounts next to them. The file would be written as:

Johnson 6000
Brown 5000
Miller 4000
Duffy 2500
Robson 1800

My code is as follows:

'''

#include <iostream>
#include <string>
#include <iomanip>
#include <fstream>
using namespace std;

void openFile();
void getData();
void computePercentages();
void sortVotes();
void display();
void displayWinner();

int main() {
    openFile();
    getData();

    return 0;
}

void openFile(){
    string fileName;
    cout << "Enter the name of the file to open: ";
    cin >> fileName;
    ifstream file;
    file.open(fileName.c_str());
}

void getData(){
    int count = 0;
    while(!file.eof()){
        string names[count];
        int votes[count];
        cin >> names[count];
        cin >> votes[count];
        count ++;
    }
}

'''

Lightness Races in Orbit
  • 358,771
  • 68
  • 593
  • 989
macb2001
  • 9
  • 1
  • 1
    Hint: you can make functions return an `ifstream` – Meowmere Nov 23 '19 at 22:48
  • By the way, [that loop is wrong](https://stackoverflow.com/q/5605125/560648) - get out of that habit now! Yes, I realise there's a 90% chance your teacher taught it to you. No, the irony of that is not lost upon me – Lightness Races in Orbit Nov 23 '19 at 23:17

3 Answers3

1

One way is to have openFile return the file stream object, then pass it to getData.

ifstream openFile()
{
    string fileName;
    cout << "Enter the name of the file to open: ";
    cin >> fileName;

    ifstream file(fileName);

    return file;
}

void getData(ifstream &file)
{
    int count = 0;
    while(file){
        string names[count];
        int votes[count];
        cin >> names[count];
        cin >> votes[count];
        count ++;
    }
}

int main()
{
    ifstream file = openFile();
    if (file)
    {
        getData(file);
    }
}

Note that this answer does not fix other issues in your code. For example, in getData you're using variable-length arrays which are non-standard and won't work on all compilers, and those arrays are constructed and destroyed each time through the while loop.

jkb
  • 2,040
  • 1
  • 5
  • 8
0

There are many ways to do it..

Here is a simple way.. using global variables.

I made ifstream file; as global..

This is not good way.. but simple..

#include <iostream>
#include <string>
#include <iomanip>
#include <fstream>
using namespace std;

void openFile();
void getData();
void computePercentages();
void sortVotes();
void display();
void displayWinner();

ifstream file;

int main() {
    openFile();
    getData();

    return 0;
}

void openFile(){
    string fileName;
    cout << "Enter the name of the file to open: ";
    cin >> fileName;
    file.open(fileName.c_str());
}

void getData(){
    int count = 0;
    while(!file.eof()){
        string names[count];
        int votes[count];
        cin >> names[count];
        cin >> votes[count];
        count ++;
    }
}
Peter Lee
  • 142
  • 8
  • `using namespace std;` and file is not closed. And many functions prototyped but never implemented. A question that is not a [mcve] doesn't an answer with has the similar problem – JHBonarius Nov 23 '19 at 22:48
0

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.

Ted Lyngmo
  • 37,764
  • 5
  • 23
  • 50