1

I have been working on a project and had no problems until today where I lost a class data member after the constructor is called. I could not pinpoint where the change happens. I have several classes for cars. The classes related to the cars are in the code block below. After all the cars are created in the class RegistrationSystem they are stored in an array. however, while calling a display function in RegistrationSystem the first car loses the color value. In the class RegistrationSystem the first Pickup in the class stores the value until the constructor is complete. After the constructor ends, the color value string shows " " in the debugger.

class Car {
protected:
    string color;

public:

    Car(string c) { color = c; };

    string getColor(void) { return color; }

    friend ostream& operator<<(ostream& outStream, const Car& car) {

        cout << car.color;
        return outStream;
    }
};

class Pickup : public Car {

public:
    Seat *frontSeat = new Seat(FRONT_SEAT_CREDIT);
    Pickup(string c) : Car(c) {}
};

class Compact : public Car {
public:
     Seat *frontSeat = new Seat(FRONT_SEAT_CREDIT);
     Seat *sideBackLeftSeat = new Seat(BACK_SEAT_COMPACT_CREDIT);
     Seat *sideBackRightSeat = new Seat(BACK_SEAT_COMPACT_CREDIT);

     Compact(string c) : Car(c) {}
};

class Sedan : public Car {
public:
    Seat *frontSeat = new Seat(FRONT_SEAT_CREDIT);
    Seat *sideBackLeftSeat = new Seat(SIDE_BACK_SEDAN_CREDIT);
    Seat *sideBackRightSeat = new Seat(SIDE_BACK_SEDAN_CREDIT);
    Seat *middleBackSeat = new Seat(MID_BACK_SEDAN_CREDIT);

    Sedan(string c) : Car(c) {}
};

class RegistrationSystem {

private:
    string file_name;
    int menu_response;

    Reservation *reservations[24] = { NULL };
    Passenger *rowers[24];
    Pickup *pickup_cars[3];
    Compact *compact_cars[3];
    Sedan *sedan_cars[3];

    // Displays the Layouts
    void displaySeatArrangements(void);
    // Saves the information in the file
    void saveToFile(void);
    // Find the rower in array rowers
    Passenger* findRower(string);
    // Displays the menu for the seat choice of a car type
    bool displayCarSeatingChoiceMenu(string);
    // Make a reservation in the system
    bool makeReservation(string, string, int, Passenger&);
    // Delete a reservation
    bool deleteReservation(int);
    // Print Reservations
    void saveReservationsToFile(void);
    // Sub functions for makeReservation()
    bool makePickupReservation(Pickup*, Passenger, int&, string, string);
    bool makeCompactReservation(Compact*, Passenger, int&, string, string);
    bool makeSedanReservation(Sedan*, Passenger, int&, string, string);

public:
    RegistrationSystem(void);
    void chooseOperation(void);
    Passenger* getPassengers(void) { return *rowers; }
    friend ostream& operator<<(ostream&, const RegistrationSystem&);
    friend istream& operator>>(istream&, RegistrationSystem&);
};

The display function has a line:

<< setw(8) << *(pickup_cars[index]) << setw(8) << *(compact_cars[index]) << setw(11) << *(sedan_cars[index]) << endl

where the *(pickup_cars[index]) is set to " " and the seat value gives error = read memory from 0x6 failed (0 of 4 bytes read). That is also set to NULL.

Here is the code of the RegistrationSystem constructor:

RegistrationSystem::RegistrationSystem(void) {
    file_name = "seat_credits.txt";
    menu_response = 0;

    ifstream inFile(file_name);

    if(!inFile.is_open()) {
        cout << "Error opening file. Terminating...";
        exit(1);
    }

    // Read file to set passengers and their credits in rowers array
    int count = 0;

    while(!inFile.eof() && count < 24) {
        string first, last;
        int credits;

        inFile >> first >> last >> credits;

        string fullName = first + ' ' + last;

        rowers[count] = new Passenger(fullName, credits);

        count++;
    }

    // Assign all the cars to the arrays
    pickup_cars[0] = new Pickup("PURPLE");
    pickup_cars[1] = new Pickup("YELLOW");
    pickup_cars[2] = new Pickup("RED");

    compact_cars[0] = new Compact("GREEN");
    compact_cars[1] = new Compact("BLUE");
    compact_cars[2] = new Compact("YELLOW");

    sedan_cars[0] = new Sedan("RED");
    sedan_cars[1] = new Sedan("GREEN");
    sedan_cars[2] = new Sedan("BLUE");

    inFile.close();
}
Andrew Serra
  • 153
  • 14
  • 1
    There is no reason to use `new` here, e.g. `Seat *frontSeat = new Seat(FRONT_SEAT_CREDIT);` should be `Seat frontSeat{FRONT_SEAT_CREDIT};`. Do you come from Java or some other language where `new` has a different use? `new` is most often bad style, here it clearly is. – walnut Nov 28 '19 at 19:23
  • 1
    Could also be, that you read from invalid memory locations. Use [std::vector](https://en.cppreference.com/w/cpp/container/vector). Then you can access elements with `.at(index)` to perform bounds checking. (Some implementations also perform bounds checking on `operator[]` in debug builds) – Lukas-T Nov 28 '19 at 19:27
  • 1
    There is nothing inherently causing the behavior in the shown code as far as I can tell. But with all the unnecessary raw pointers and dynamic memory allocation you are sure to make a mistake somewhere sooner or later. Your classes are also all leaking their memory because of how you use `new`. Please produce a [repro]. – walnut Nov 28 '19 at 19:29
  • "*That is also set to NULL.*": What does that mean? You know that you can't dereference null pointers, right? You are setting `pickup_cars[index]` to a valid pointer before using it in the line shown, I hope? – walnut Nov 28 '19 at 19:33

1 Answers1

0

The problem occurred when the input file stream was still open and I was creating new instances and assigning their addresses to pointers before I closed the file. After I switched the order and closed the file, then assigned pointers the addresses of the new objects the problem went away.

These operations were happening inside of the constructor of the RegistrationSystem class. Here is the new code:

RegistrationSystem::RegistrationSystem(void) {
    file_name = "seat_credits.txt";
    menu_response = 0;

    ifstream inFile(file_name);

    if(!inFile.is_open()) {
        cout << "Error opening file. Terminating...";
        exit(1);
    }
    // Read file to set passengers and their credits in rowers array
    int count = 0;

    while(!inFile.eof() && count < 24) {
        string first, last;
        int credits;

        inFile >> first >> last >> credits;
        string fullName = first + ' ' + last;
        rowers[count] = new Passenger(fullName, credits);
        count++;
    }
    inFile.close();

    // Assign all the cars to the arrays
    pickup_cars[0] = new Pickup("PURPLE");
    pickup_cars[1] = new Pickup("YELLOW");
    pickup_cars[2] = new Pickup("RED");

    compact_cars[0] = new Compact("GREEN");
    compact_cars[1] = new Compact("BLUE");
    compact_cars[2] = new Compact("YELLOW");

    sedan_cars[0] = new Sedan("RED");
    sedan_cars[1] = new Sedan("GREEN");
    sedan_cars[2] = new Sedan("BLUE");
}

The line inFile.close() was placed after all the array assignments. Moving it above them solved the problem of losing the first data member of my Pickup object.

Andrew Serra
  • 153
  • 14