vector .pushback method not pushing class object into vector due to copy constructor

Question

I have a class cashierwhich has 3 attributres : ID , Password ,tries with the standard GET & SET method

//Header file

#ifndef CASHIER_H
#define CASHIER_H
#include <string>
using namespace std;


class cashier 
{
public:
    cashier();
    cashier(const cashier& orig);
    virtual ~cashier();

        void setID(string);
    string getID();

    void setPassword(string);
    string getPassword();

    void settries(int);
    int gettries();

private:
    string ID;
    string Password;
    int tries;



};

#endif  /* CASHIER_H */

cashier.cpp file

#include "cashier.h"

cashier::cashier() 
{

}

cashier::cashier(const cashier& orig) 
{

}

cashier::~cashier() 
{

}

void cashier::setID(string value)
{
    this->ID = value;
}

void cashier::setPassword(string value)
{
    this->Password = value;
}

string cashier::getID()
{
    return this->ID;
}

string cashier::getPassword()
{
    return this->Password;
}

void cashier::settries(int value)
{
    this->tries=value;
}
int cashier::gettries()
{
    return this->tries;
}

In my main file , I am attempting to read from a text file and store the values inside cashier c and push it into my vector cashier_all

#include <iostream>
#include "cashier.h"
#include <fstream>
#include <vector>
int main()
{

fstream afile;
        char rubbish[100];

        afile.open("cashier.txt",ios::in);

        afile.getline(rubbish,100); //read in first line

        vector <cashier> cashier_all;

            cashier c;
            string temp_id;
            string temp_password;
            int temp_tries;


        while(afile>>temp_id)
        {
            afile>>temp_password;

            afile>>temp_tries;

            c.setID(temp_id);
            c.setPassword(temp_password);
            c.settries(temp_tries);

            cashier_all.push_back(c); //c is not being pushed into the vector
                                      // for some unknown reason


        }

            vector<cashier>::iterator v1;
            vector<cashier>::iterator v2;
            v1 = cashier_all.begin();
            v2 = cashier_all.end();

            while (v1 != v2)
            {
                cout<<v1->getID()
                    <<endl
                    <<v1->getPassword()
                    <<endl
                    <<v1->gettries();
                v1++;


            }

            system("PAUSE");

}

cashier.txt

CashierID               password        tries
001                     def             0
002                     ghi         0
003                     jkl         0

Checking my debugger , the error is at the cashier_all.pushback where it is not pushing c into the vector cashier_all , You can try for yourself if you dont believe me

EDIT : It works after i remove all the constructors

i dont understand why copy constructor would affect the pushing of cashier into cashier_all , can someone explain to me ??

@DietmarKühl why does the copy constructor affect the code?? It works after i remove all the constructors — Computernerd, Jan 15 '14 at 02:46

score 3 · Accepted Answer · answered Jan 15 '14 at 02:48

3

Get rid of unnecessary implementation of generated methods, e.g., your copy constructor: as is, your copy constructor does the same as the default constructor. It should, however, copy the object. Removing it will result in a copy constructor which does just that. As an added bonus, the compiler would also generate a move constructor for you.

When inserting an object into a container the object is copied (or moved if it is a temporary object): C++ is based on values, not on references!

answered Jan 15 '14 at 02:48

Dietmar Kühl

150,225
13
225
380

what should the implementation of my copy constructor be for it to work ?? – Computernerd Jan 15 '14 at 02:59
1

@Computernerd: If you implement a copy constructor your code has to actually __copy__ the members. Yours was an empty function. – Blastfurnace Jan 15 '14 at 03:03
@Computernerd, to restate what others are saying, your implementation should be *absent*. Don't declare or implement a copy constructor and the compiler will generate one for you that performs a member-wise copy. Your empty implementation supplants the default behavior and does nothing. – sbaker Jan 15 '14 at 20:31

score 3 · Answer 2 · answered Jan 15 '14 at 05:06

3

Your current copy constructor is not really copying the object. A decent implementation would be:

cashier::cashier(const cashier& orig) 
    : ID(origin.ID)
    , Password(origin.Password)
    , tries(origin.tries)
    {}

but C++ defines this kind of constructor by default, therefore you don't need to write one. The same goes for your default constructor and your destructor.

I'm also pretty scared by the:

with the standard GET & SET method

phrase you mentioned there. There's no such a thing as a standard GET and SET method. Your member functions have absolutely no sense to exists.

In this case you can just use a struct:

struct cashier {
    std::string ID;
    std::string Password;
    int tries;
};

Can you see the beauty of C++, now? Remember that encapsulation does not mean: add a bunch of getters and setters.

answered Jan 15 '14 at 05:06

Shoe

74,840
36
166
272

I disagree with this. I agree that it is incorrect to refer to a "standard" GET and SET, but I don't agree with the statement that his member functions have no purpose. He may need to implement bounds checking or something like that in his modifier and using a struct would not allow him to add anything like that without changing the interface. I'm not trying to say that the example is good design, just that his member functions could serve a valid purpose. – sbaker Jan 15 '14 at 05:55
@sbaker, please read [this magnificent answer](http://stackoverflow.com/questions/1568091/why-use-getters-and-setters/12108025#12108025) about that silly idea. – Shoe Jan 15 '14 at 06:04
that magnificent answer does not accommodate the possibility for incremental development. To reiterate, I am not defending the design in the OP question - I very rarely provide simple get/set functions for member variables. But there is an advantage over using a struct in that invariants can be defined/changed after the fact without requiring changes to all the client code. I agree with your sentiment regarding setters/getters, but I don't agree that they are equivalent to using a struct. – sbaker Jan 15 '14 at 20:28

vector .pushback method not pushing class object into vector due to copy constructor

2 Answers2