Cannot Read & Store Data From File

[PiggiesGoSqueal]

Please Note: This is for a homework assignment, however, the due date has already passed. I'm doing this for the learning aspect only.

I'm trying to read from a file and store the data into variables (then later into a class using an overload constructor). However, it won't work. I double checked Duthomhas's post here: http://www.cplusplus.com/forum/beginner/250853/#msg1104657 about cin.ignore(1000, '\n'); to make sure I had it correct for iFile.ignore(1000, '\n'); and it appears I do so I'm quite confused why it isn't working.

There are 5 variables: player name, team name, points, rebounds, and assists.
The class that stores these member variables is called Player. I created a vector called Players of type Player. I'm trying to read from a player's inputted file name, store the data into temporary variables, then use the overload constructor of the class to store the data into the class.

Current Output:

Successfully opened players.txt!
Stephen Curry
5008en State Warriors

players.txt:

Stephen Curry
Golden State Warriors
29.8
5.2
6.5
Lebron James
Cleveland Cavaliers
25.7
7.4
6

Main.cpp:

#include "Player.h"
#include "Main.h"

int main() {
	vector<Player> Players;

	// Call functions from Main.h
	readFile(Players);

	return 0;
}

Main.h:

// Main.h
// Includes all functions called in Main.cpp
// Includes all #includes necessary for this file and Main.cpp
#include <iostream>
#include <string>
#include <vector>
#include <fstream>
#include <stdlib.h>
using namespace std;

// Guard:
#ifndef MAIN_H
#define MAIN_H

// Reads file, stores data into class, then closes file.
void readFile(vector<Player> &PlayersVec) {
    ifstream iFile;
    string fileName;
    // Temp File Data Variables:
    string pName, tName;
    int pnts, rbnds, assts; // points, rebounds, assists

    // Asks for user to input file name.
    cout << "Please enter input file name: ";
    cin >> fileName;

    // Opens file name
    iFile.open(fileName);

    // If file cannot be opened, print error and exit.
    if (!iFile) {
        cerr << "Error! Could not open input file!\n";
        exit(EXIT_FAILURE);
    }
    cout << "Successfully opened " << fileName << "!\n";

    getline(iFile, pName);
    cout << pName << endl;
    
    // Goes through every file line and stores info.
    while (getline(iFile, pName)) {
        getline(iFile, tName);
        iFile >> pnts >> rbnds >> assts;
        iFile.ignore(1000, '\n');

        cout << pName << tName << pnts << rbnds << assts;

	    // Sets data in class using overload constructor instead
	    // of default.
        Player PlayersVec(pName, tName, pnts, rbnds, assts);
    }
    cout << "\n";

    // Closes input file when done using it.
    iFile.close();
}

#endif 

Player.h: (Class)

// Player.h
#include <iostream>
#include <string>
using namespace std;

// Guard:
#ifndef PLAYER_H
#define PLAYER_H

class Player {
    private:
        string playerName;
        string teamName;
        int points;
        int rebounds;
        int assists;

    public:
        // default constructor
        // sets all member variables to empty / zero.
        Player();

        // Overload constructor
        Player(string, string, int, int, int);

        // destructor
        ~Player();

        // Accessor Functions
        string getPName() const;
            // getPName - returns name of player

        string getTName() const;
            // getTName - returns team name

        int getPnts() const;
            // getPnts - returns points

        int getRbnds() const;
            // getRbnds - returns rebounds

        int getAssts() const;
            // getAssts - returns assists

        // Mutator Functions
        void setPName(string);
            // setPName - sets player name
            // @param string - name of player
        
        void setTName(string);
            // setTName - sets team name
            // @param string - name of team
        
        void setPnts(int);
            // setPnts - sets points
            // @param int - points
        
        void setRbnds(int);
            // setRbnds - sets rebounds
            // @param int - rebounds
        
        void setAssts(int);
            // setAssts - sets assists
            // @param int - assists
};

#endif 

Player.cpp (Class Methods)

#include "Player.h"

// Default constructor
Player::Player() {
    playerName = "";
    teamName = "";
    points = 0;
    rebounds = 0;
    assists = 0;
}

// Overload constructor
Player::Player(string pName, string tName, int pnts, int rbnds, int assts) {
    playerName = pName;
    teamName = tName;
    points = pnts;
    rebounds = rbnds;
    assists = assts;
}

// Destructor
// Should it be empty?????
Player::~Player() {

}

// Accessor Functions
string Player::getPName() const {  return playerName;  }
    // getPName - returns player name

string Player::getTName() const {  return teamName;  }
    // getTName - returns team name

int Player::getPnts() const {  return points;  }
    // getPnts - returns points

int Player::getRbnds() const {  return rebounds;  }
    // getRbnds - returns rebounds

int Player::getAssts() const {  return assists;  }
    // getAssts - returns assists

// Mutator Functions
void Player::setPName(string pName) {
    playerName = pName;
}

void Player::setTName(string tName) {
    teamName = tName;
}

void Player::setPnts(int pnts) {
    points = pnts;
}

void Player::setRbnds(int rbnds) {
    rebounds = rbnds;
}

void Player::setAssts(int assts) {
    assists = assts;
}

Help would be greatly appreciated. Thank you!

[ne555]

several things:
- you have an extra getline() before the start of the loop, that will offset the input and make it fail.

- points, rebounds, assists, should be double instead of int
same for pnts, rbnds, assts, (¿are they charging you for vowels?)

- your "text" file was created in a different operating system than the one you are testing.
there the line end is marked with "\r\n" so you end up reading the '\r' character as a part of the string (use a debugger to watch your variables)

- you never update the vector, instead you create a local variable with the same name as your parameter
instead do PlayersVec.emplace_back(pName, tName, pnts, rbnds, assts);

[Handy Andy]

Hello PiggiesGoSqueal,

"main.h" if you are going to do something like this I would only put lines 4 - 8 in this file. Line 9 should never be used in a header file. You should read this: http://www.lonecpluspluscoder.com/2012/09/22/i-dont-want-to-see-another-using-namespace-xxx-in-a-header-file-ever-again/

Although it may seem easy right now and it may work for now, but putting a function like "readFile" in a header file is not the best way to do this. for a function like "readFile" this would be better put in a file like "readFile.cpp".

That said what i first see is void readFile(vector<Player> &PlayersVec)then on line 50 you have
Player PlayersVec(pName, tName, pnts, rbnds, assts); I see a possible conflict in in the naming here, What I do not see is where you add this newly constructed object of "Players" into the vector. I believe what you want is:

Player temp(pName, tName, pnts, rbnds, assts);
playersVec.emplace_back(temp);

Just because you create an object of "Player" with the same name as the vector does not put it in the vector.

Shortly I will load this and give it a test run and see what happens.

Hope that helps,

Andy

[PiggiesGoSqueal]

Thanks for your reply Andy!

I will try to remember to refrain from using "using namespace std;" in header files from now on. I've converted all my current files to not have that line in them and use "std::" where needed instead. I was originally using it because it makes it easier to focus on / learn new concepts like map, vector, etc.

Also, is it generally better to have a separate file for each function or can I have all functions used in Main.cpp be in 1 file like MainFunctions.cpp? I'm compiling it via command prompt g++ so I'd need to do g++ -std=c++14 Main.cpp <all other .cpp file names> which would probably become quite a list with all functions in separate files.

I tried moving the readFile function from a header file to MainFunctions.cpp but when I compiled the files, it gave the error:

../NBA-OOP$ g++ -std=c++14 MainFunctions.cpp Player.cpp Main.cpp
Main.cpp: In function ‘int main()’:
Main.cpp:74:2: error: ‘readFile’ was not declared in this scope
  readFile(Players);
  ^~~~~~~~

...
I believe what you want is:

Player temp(pName, tName, pnts, rbnds, assts);
playersVec.emplace_back(temp);

Ah yeah I see what you mean. I had to do a similar thing for an assignment using struct (instead of class) except I used push_back() instead of emplace_back(). What is the difference between the two? Is it okay to use push_back()?

Main.cpp:

#include <iostream>
#include <vector>
#include "Player.h"
// All functions used in Main.cpp are in MainFunctions.cpp


int main() {
	std::vector<Player> Players;

	// Call functions from Main.h
	readFile(Players);

	return 0;
}

MainFunctions.cpp:

#include <iostream>
#include <string>
#include <vector>
#include <fstream> // file reading & writing
#include <stdlib.h> // for exit() & EXIT_FAILURE

#include "Player.h"

// Reads file, stores data into class, then closes file.
void readFile(std::vector<Player> &PlayersVec) {
    std::ifstream iFile;
    std::string fileName;
    // Temp File Data Variables:
    std::string pName, tName;
    int pnts, rbnds, assts;

    // Asks for user to input file name.
    std::cout << "Please enter input file name: ";
    std::cin >> fileName;

    // Opens file name
    iFile.open(fileName);

    // If file cannot be opened, print error and exit.
    if (!iFile) {
        std::cerr << "Error! Could not open input file!\n";
        exit(EXIT_FAILURE);
    }
    std::cout << "Successfully opened " << fileName << "!\n";

    std::getline(iFile, pName);
    std::cout << pName << '\n';
    
    // Goes through every file line and stores info.
    while (std::getline(iFile, pName)) {
        std::getline(iFile, tName);
        iFile >> pnts >> rbnds >> assts;
        iFile.ignore(1000, '\n');

        std::cout << pName << tName << pnts << rbnds << assts;

	// Sets data in class using overload constructor instead
	// of default.
        Player temp(pName, tName, pnts, rbnds, assts);
        PlayersVec.emplace_back(temp);
    }
    std::cout << "\n";
    

    // Closes input file when done using it.
    iFile.close();
}

Player.h:

// Player.h
#include <iostream>
#include <string>

// Guard:
#ifndef PLAYER_H
#define PLAYER_H


class Player {
    private:
        std::string playerName;
        std::string teamName;
        int points;
        int rebounds;
        int assists;

    public:
        // default constructor
        // sets all member variables to empty / zero.
        Player();

        // Overload constructor
        Player(std::string, std::string, int, int, int);

        // destructor
        ~Player();

        // Accessor Functions
        std::string getPName() const;
            // getPName - returns name of player

        std::string getTName() const;
            // getTName - returns team name

        int getPnts() const;
            // getPnts - returns points

        int getRbnds() const;
            // getRbnds - returns rebounds

        int getAssts() const;
            // getAssts - returns assists

        // Mutator Functions
        void setPName(std::string);
            // setPName - sets player name
            // @param string - name of player
        
        void setTName(std::string);
            // setTName - sets team name
            // @param string - name of team
        
        void setPnts(int);
            // setPnts - sets points
            // @param int - points
        
        void setRbnds(int);
            // setRbnds - sets rebounds
            // @param int - rebounds
        
        void setAssts(int);
            // setAssts - sets assists
            // @param int - assists
};

#endif 

Player.cpp:

#include "Player.h"

// Default constructor
// Sets everything to empty / zero.
Player::Player() {
    playerName = "";
    teamName = "";
    points = 0;
    rebounds = 0;
    assists = 0;
}

// Overload constructor
// Sets everything to specific values.
Player::Player(std::string pName, std::string tName, int pnts, int rbnds, int assts) {
    playerName = pName;
    teamName = tName;
    points = pnts;
    rebounds = rbnds;
    assists = assts;
}

// Destructor
// Should it be empty?????
Player::~Player() {

}

// Accessor Functions
std::string Player::getPName() const {  return playerName;  }
    // getPName - returns player name

std::string Player::getTName() const {  return teamName;  }
    // getTName - returns team name

int Player::getPnts() const {  return points;  }
    // getPnts - returns points

int Player::getRbnds() const {  return rebounds;  }
    // getRbnds - returns rebounds

int Player::getAssts() const {  return assists;  }
    // getAssts - returns assists

// Mutator Functions
void Player::setPName(std::string pName) {
    playerName = pName;
}

void Player::setTName(std::string tName) {
    teamName = tName;
}

void Player::setPnts(int pnts) {
    points = pnts;
}

void Player::setRbnds(int rbnds) {
    rebounds = rbnds;
}

void Player::setAssts(int assts) {
    assists = assts;
}

[Handy Andy]

Hello PiggiesGoSqueal,

Much nicer and a better format.

As ne555 has pointed out the variables that hold "points", "rebounds" and "assists" should all be "double"s not "int"s. It does make a difference.

To ne555 thank you I did not realize that "emplace_back" could be used that way.

PiggiesGoSqueal if you use ne555's suggestion you can shorten my two line to one.

Andy

[PiggiesGoSqueal]

Oh, oops. Sorry @ne555. I didn't see your response.

Thank you ne555 for responding!

I didn't notice the getline() before the loop, thanks!

- points, rebounds, assists, should be double instead of int
same for pnts, rbnds, assts, (¿are they charging you for vowels?)

Ohh right, I totally overlooked the fact the input file was using doubles not ints... Whoops. Thank you both (ne555 & Andy) for pointing that out haha.

I took out the vowels because if I called them the same names as the member variables of the class, that doesn't mean I'd actually be using those variables, they'd just have the same name but different values which is confusing. Thus I named it something else, and I chose to take out the vowels because I felt that made more sense than any other name I could come up with.

- your "text" file was created in a different operating system than the one you are testing.
there the line end is marked with "\r\n" so you end up reading the '\r' character as a part of the string (use a debugger to watch your variables)

I don't understand what you mean? I'm using Windows 10 with WSL (Windows Sub Linux) to use Ubuntu. What's that have to do with the .txt file though? Also, I've never seen an \r so I'm not sure what you mean.

----------------------------------
Thank you for your reply Andy!

Thanks for pointing out what ne555 said, I didn't see their post until now, oops.

I've made the changes you both mentioned but I'm still getting the same error for some reason.

Output:

Main.cpp: In function ‘int main()’:
Main.cpp:69:2: error: ‘readFile’ was not declared in this scope
  readFile(Players);
  ^~~~~~~~

I originally used a header for the readFile() function because I know headers are inserted into the file when compiled so Main.cpp would have access to all functions in the header. I'm not as familiar with what happens when you put functions in a .cpp file and link them using the command:
g++ -std=c++14 Main.cpp MainFunctions.cpp Player.cpp
So I suspect the issue has something to do with that.

By the way, is it not possible to use my overload constructor instead of the line:
PlayersVec.emplace_back(pName, tName, pnts, rbnds, assts); from MainFunctions.cpp? I'm still learning constructors/destructors so it'd be good practice to use the overload constructor but not required.

Lastly, is my destructor in Player.cpp correct? It's just empty like a tutorial I watched on YouTube:

Player::~Player() {

}

Main.cpp:

#include <iostream>
#include <vector>
#include "Player.h"
// All functions used in Main.cpp are in MainFunctions.cpp

int main() {
	std::vector<Player> Players;

	// Call function from MainFunctions.cpp
	readFile(Players);

	return 0;
}

MainFunctions.cpp:

#include <iostream>
#include <string>
#include <vector>
#include <fstream> // file reading & writing
#include <stdlib.h> // for exit() & EXIT_FAILURE

#include "Player.h"

// Reads file, stores data into class, then closes file.
void readFile(std::vector<Player> &PlayersVec) {
    std::ifstream iFile;
    std::string fileName;
    // Temp File Data Variables:
    std::string pName, tName;
    double pnts, rbnds, assts;

    // Asks for user to input file name.
    std::cout << "Please enter input file name: ";
    std::cin >> fileName;

    // Opens file name
    iFile.open(fileName);

    // If file cannot be opened, print error and exit.
    if (!iFile) {
        std::cerr << "Error! Could not open input file!\n";
        exit(EXIT_FAILURE);
    }
    std::cout << "Successfully opened " << fileName << "!\n";

    
    // Goes through every file line and stores info.
    while (std::getline(iFile, pName)) {
        std::getline(iFile, tName);
        iFile >> pnts >> rbnds >> assts;
        iFile.ignore(1000, '\n');

        std::cout << pName << tName << pnts << rbnds << assts;

        PlayersVec.emplace_back(pName, tName, pnts, rbnds, assts);
    }
    std::cout << "\n";
    

    // Closes input file when done using it.
    iFile.close();
}

Player.h:

// Player.h
#include <iostream>
#include <string>

// Guard:
// (Not compiler dependent guard type, that is good)
#ifndef PLAYER_H
#define PLAYER_H

class Player {
    private:
        std::string playerName;
        std::string teamName;
        double points;
        double rebounds;
        double assists;

    public:
        // default constructor
        // sets all member variables to empty / zero.
        Player();

        // Overload constructor
        Player(std::string, std::string, double, double, double);

        // destructor
        ~Player();

        // Accessor Functions
        std::string getPName() const;
            // getPName - returns name of player

        std::string getTName() const;
            // getTName - returns team name

        double getPnts() const;
            // getPnts - returns points

        double getRbnds() const;
            // getRbnds - returns rebounds

        double getAssts() const;
            // getAssts - returns assists

        // Mutator Functions
        void setPName(std::string);
            // setPName - sets player name
            // @param string - name of player
        
        void setTName(std::string);
            // setTName - sets team name
            // @param string - name of team
        
        void setPnts(double);
            // setPnts - sets points
            // @param double - points
        
        void setRbnds(double);
            // setRbnds - sets rebounds
            // @param double - rebounds
        
        void setAssts(double);
            // setAssts - sets assists
            // @param double - assists 

Player.cpp:

#include "Player.h"

// Default constructor
Player::Player() {
    playerName = "";
    teamName = "";
    points = 0;
    rebounds = 0;
    assists = 0;
}

// Overload constructor
Player::Player(std::string pName, std::string tName, double pnts, double rbnds, double assts) {
    playerName = pName;
    teamName = tName;
    points = pnts;
    rebounds = rbnds;
    assists = assts;
}

// Destructor
// Should it be empty?????
Player::~Player() {

}

// Accessor Functions
std::string Player::getPName() const {  return playerName;  }
    // getPName - returns player name

std::string Player::getTName() const {  return teamName;  }
    // getTName - returns team name

double Player::getPnts() const {  return points;  }
    // getPnts - returns points

double Player::getRbnds() const {  return rebounds;  }
    // getRbnds - returns rebounds

double Player::getAssts() const {  return assists;  }
    // getAssts - returns assists

// Mutator Functions
void Player::setPName(std::string pName) {
    playerName = pName;
}

void Player::setTName(std::string tName) {
    teamName = tName;
}

void Player::setPnts(double pnts) {
    points = pnts;
}

void Player::setRbnds(double rbnds) {
    rebounds = rbnds;
}

void Player::setAssts(double assts) {
    assists = assts;
}

Thank you!

[ne555]

> ‘readFile’ was not declared in this scope
before you use a function you need to see its prototype
void readFile(std::vector<Player> &PlayersVec);

> Also, I've never seen an \r so I'm not sure what you mean.
your output was

5008en State Warriors

actually it had written `Golden State Warriors' then went back to the beginning of the line, wrote 29.8, then back againg and put 5, 0, 0
if you observe your variables in a debugger you'll see that

pName -> "Golden State Warriors\r"
tName -> "29.8\r"
pnts -> 5

and when you "print" '\r' it'll send the cursor to the beginning of the line

that occurs because windows uses "\r\n" as a line break separator but linux uses only "\n"

[PiggiesGoSqueal]

Thanks for your reply @ne555! Sorry for my late reply.

> ‘readFile’ was not declared in this scope
before you use a function you need to see its prototype
void readFile(std::vector<Player> &PlayersVec);

Ah okay, I didn't realize I needed to reference the function if it's used in another .cpp file. Added!

...
that occurs because windows uses "\r\n" as a line break separator but linux uses only "\n"

Ahh okay. Huh, strange that I've never ran into an issue with that before. Thanks for explaining it! That fixed it!

My new output is:

[Debug]
player name = Lebron James
team name = Cleveland Cavaliers
points = 25.7
rebounds = 7.4
assists = 6

I believe all of my issues have been explained / solved so I'm marking this post as solved. Thanks everyone that responded!
I also added this to my bookmarks as a reminder for the future about "\r\n"!