Push_back not storing data to vector

For some reason my vector doesn't push_back data (or accept data)

Code:

---


class Fleet
{
private:

        typedef vector<Car> CarFleet;
        typedef CarFleet::iterator FleetIterator;

public:

        Fleet();
        void ReadFleetFromFile();

private:
        CarFleet fleet;
};

---

Code:

---

void Fleet::ReadFleetFromFile()
{                                       
        char filename[MAX_PATH] ;

        puts("\n\nPlease enter the text file to open:\n");
        cin >> filename;

        ifstream fleetin;
        fleetin.open (filename);

        /* Ensure file was opened, otherwise send an error */
        if ( fleetin.fail() )
        {
                puts("");
                perror("ERROR! while trying to open file");
                exit(1);
        }
        else
        {
                // Read number of records from file
                while (!fleetin.eof())
                {
                        Car car;       

                        if( car.Read(fleetin) )
                                fleet.push_back(car); // After this line car contains data, but fleet doesn't
                }
                puts("\nProcessed reading...");               
        }

        fleetin.close();

}

---

I used a debug (F5) and a break point at fleet.push_back(car) ... Car does contain data read from file, after executing this line, by using VS2005 watch window on fleet, i just see {_Myfirst='some address' _Mylast='address' Myend='address again'} ... An when i expoand the plus sign on fleet watch window to check elements, there are 'error' with 0 values... Completely lost here

May or may not be relevant but... The STL containers make extensive use of copy constructors when inserting/pushing data into them. What does you Car class look like? If it contains dynamically allocated data, then it may need a custom copy constructor in order for this to work properly.

I'd like to see the car.Read() function, my guess is that it is returning false and so fleet.push_back is never getting called.

Even if you you don't have a custom copy constructor(but needed one), the default constructor will push something back, so it shouldn't be empty.

Quote:

---

Originally Posted by Darryl

I'd like to see the car.Read() function, my guess is that it is returning false and so fleet.push_back is never getting called.

Even if you you don't have a custom copy constructor(but needed one), the default constructor will push something back, so it shouldn't be empty.

---

Yes, it would not have even compiled unless there was a copy constructor (whether default or not), so I think the problem is not related to push_back..

On a design note, I don't usually like functions like X.read(). What does a Car have to do with a stream? I prefer external friend functions for IO on classes.

Quote:

---

Originally Posted by Darryl

I'd like to see the car.Read() function, my guess is that it is returning false and so fleet.push_back is never getting called.

Even if you you don't have a custom copy constructor(but needed one), the default constructor will push something back, so it shouldn't be empty.

---

Code:

---


// Read fleet data from either keyboard or file
bool Car::Read(istream& indata)
{
        if ( !(indata >> *this) )
                return false;
        return true;
}

// Overload for reading input file streams
istream& operator>>(istream& in, Car& car)
{
        string make, model, regNo;
        int ymodel, engCap;
        Car::TankStatus ts;

        in >> make >> model;
        in >> regNo;
        in >> ymodel;
        in >> engCap;
        in >> ts;
       
        // Ensure correct tank details
        if ( (ts>3) || (ts<0) ) {
                ts=Car::unknown;
        }

        car.setCarDetails(make, model, regNo, ymodel, engCap, ts);

        return in;
}

---

Don't use the debugger to determine the vector's size. Call fleet.size() and look at the output of that function.

>> while (!fleetin.eof())
This won't help, or will cause problems. Use:

Code:

---

Car car;       
while (fleetin >> car)
        fleet.push_back(car);

---

You should probably also fix your operator>> so that it checks the state of in before it calls setCarDetails, although I'm not sure it matters in this case.

If you're still having trouble, put a breakpoint in your operator>> and watch it read the data in, does that work?

Quote:

---

Originally Posted by Daved

Don't use the debugger to determine the vector's size. Call fleet.size() and look at the output of that function.

>> while (!fleetin.eof())
This won't help, or will cause problems. Use:

Code:

---

Car car;       
while (fleetin >> car)
        fleet.push_back(car);

---

---

I moved this code from Fleet to Car

Code:

---

// Read fleet data from either keyboard or file
int Car::Read(istream& indata)
{
        // Read each record
        while ( indata >> *this ) {       
                fleet.push_back(*this);
        }
        return (int) fleet.size();
}

---

Code:

---

//Back to fleet : where read is called ... (in Fleet.cpp)

Car car;       
               
if ( car.Read(fleetin) )
{
        puts("\nProcessed reading...");
        SortAlpha();               
}
else
puts("\nData was unsuccessfully read !!...");

---

But now Car cannot see Fleet "flee.push_back" ... If i try to include fleet.h inside Car.h, there's a problem because fleet includes car already.. ": error C2011: 'Car' : 'class' type redefinition"

Quote:

---

Originally Posted by csonx_p

I moved this code from Fleet to Car

Code:

---

// Read fleet data from either keyboard or file
int Car::Read(istream& indata)
{
        // Read each record
        while ( indata >> *this ) {       
                fleet.push_back(*this);
        }
        return (int) fleet.size();
}

---

Code:

---

//Back to fleet : where read is called ... (in Fleet.cpp)

Car car;       
               
if ( car.Read(fleetin) )
{
        puts("\nProcessed reading...");
        SortAlpha();               
}
else
puts("\nData was unsuccessfully read !!...");

---

But now Car cannot see Fleet "flee.push_back" ... If i try to include fleet.h inside Car.h, there's a problem because fleet includes car already.. ": error C2011: 'Car' : 'class' type redefinition"

---

k, i did a forward declaration in Fleet.h (class Car) since i'm only declaring Car as a vector in Fleet. I then included Fleet.h in car.h. After doing this in car

Code:

---

Fleet::fleet.push_back(*this);

---

i get : error C2228: left of '.push_back' must have class/struct/union ???

Your original approach was right. The Cars should be created in a read function of fleet.
The problem you have now is that a car doesn't have a member fleet. That's why you cannot call

Code:

---

fleet.push_back(*this);

---

from inside Car::Read().
You could pass a pointer or reference to the fleet the car belongs to, in the constructor of car.
Kurt

>> I moved this code from Fleet to Car
Why? I liked it better the other way. A Car should not know about a Fleet.

Quote:

---

Originally Posted by Daved

>> I moved this code from Fleet to Car
Why? I liked it better the other way. A Car should not know about a Fleet.

---

That's right, moved it back... In fact i have spent some time with a friend looking at the code, and he hammered me with lots of things...

1. A header file should not include other interfaces. There's virtually no need to have #includes in any header file, unless it can't do with forward declarations
2. A .cpp file should always start by including it's .h file before including other libraries

These two things has thought me a lot and solved a lot of my problems...


Now i have a new silly problem, have a look at this code...

Code:

---

// Match  the right car: Many of the same car could exist
// Therefore, match them using a car registration...
bool Fleet::matchRightCar( FleetIterator iter )
{
        bool entered = false;
        string carReg;

        // Keep comparing the current & next matching car
        for
        (        ;
                iter->getMake()==(iter+1)->getMake() &&               
                iter->getModel()==(iter+1)->getModel() &&       
                (iter+1) <= fleet.end()                                               
                ;                                                                                       
                iter+=2        // Jump two cars                                                               
        )
        {
                // Check if entered registration already
                if(!entered)
                {
                        cout << "\n\nMore than one car found!";
                        cout << "....Enter Car Registration : ";
                        cin >> carReg;
                        entered=true;
                }

                // Find right car based on Registration
                if( carReg == iter->getRegistration() )        {
                        return true;
                }
                else
                if( carReg == (iter+1)->getRegistration() )
                {
                        iter++;
                        return true;
                }
        }

        return false;
}

---

I get a RUN TIME error : Vector iterator dereferencable... By using a break point, figured the problem is when i increment the vector in the for loop ... Doesn't make sense

Suppose that iter points to the last element. Now, iter+1 <= fleet.end(), since iter+1 == fleet.end(). But you compare iter->getMake() with (iter+1)->getMake() and iter->getModel() with (iter+1)->getModel(), thus dereferencing the one past the end iterator twice. A possible solution is to change it to:

Code:

---

        for
        (        ;
                (iter+1) != fleet.end() &&
                iter->getMake() == (iter+1)->getMake() &&
                iter->getModel() == (iter+1)->getModel()
                ;
                ++iter;
        )

---

Quote:

---

Originally Posted by laserlight

Suppose that iter points to the last element. Now, iter+1 <= fleet.end(), since iter+1 == fleet.end(). But you compare iter->getMake() with (iter+1)->getMake() and iter->getModel() with (iter+1)->getModel(), thus dereferencing the one past the end iterator twice. A possible solution is to change it to:

Code:

---

        for
        (        ;
                (iter+1) != fleet.end() &&
                iter->getMake() == (iter+1)->getMake() &&
                iter->getModel() == (iter+1)->getModel()
                ;
                ++iter;
        )

---

---

cool, solved the error

By the way, by using (iter+1) and iter+=2, you are restricting FleetIterator to be a random access iterator. It is fine for now, but if you want your implementation to be more future proof, you might create a next iterator and use advance() instead.