0

I am trying to get my code to display the Ship class data, then use pointers to display the updated data after enter in console and set to the relevant variables in the CruiseShip and CragoShip classes. However, the program will ignore any new input and simply display the base class data again regardless of new data.

What is missing here?


    using namespace std;

    // -- Initialized the class 'Ship' -- //

    class Ship {

    public:

        int yearBuilt;
        string nameOfShip;

        // Constructs the ship class and defines the values of the variables
        Ship() {
            yearBuilt = 0;
            nameOfShip = "";
        }
        // Overloaded Constructor
        Ship(int year, string name) {

            Ship::yearBuilt = year;
            Ship::nameOfShip = name;

            year = 0;
            name = "";
        }
        // Accessor Function for the year the ship was built
        int getYearBuilt() const {
            return yearBuilt;
        }
        // Accessor Function for the name of the ship
        string getShipName() const {
            return nameOfShip;
        }
        // Mutator Function to read input and set it to the value of yearBuilt
        void setShipYear(int year) {
            cout << " Enter Year: " << endl;
            cin >> year;
            year = yearBuilt;

            cout << endl; // Spacing between printed data
        }
        // Mutator Function to read input and set it to the value of nameOfShip
        void setShipName(string name) {
            cout << " Enter Name: " << endl;
            cin >> name;
            name = nameOfShip;

            cout << endl; // Spacing between printed data
        }
        // Virtual Print function to display the name of the ship and the year it was built
        virtual void print() const {

            cout << " Ship Name: " << nameOfShip << endl;
            cout << " Year Built: " << yearBuilt << endl;
        }
    };

    // -- Initializes the class 'CruiseShip' derived from Ship class -- //

    class CruiseShip : public Ship
    {

    // Set the member variable for max number of passengers

    int passengersMAX;

    public:

        //Constructor for CruiseShip, calls parent class

        CruiseShip() : Ship() {

            passengersMAX = 0;
        }

        //Overloaded Constructor
        CruiseShip(int maximum, int year, string name) : Ship() {
            CruiseShip::passengersMAX = maximum;
        }

        //Accessor
        int getMaxPass() const {
            return passengersMAX;
        }

        //Mutator
        void setMaxPass(int maximum) {
            cout << "Enter Passenger Max: " << endl;
            cin >> maximum;
            maximum = passengersMAX;
        }

        //Overriding Print Function
        virtual void print() const override{
            cout << " Ship Name: " << nameOfShip << endl;
            cout << " Max number Of Passengers: " << passengersMAX << endl;
        }
    };

    class CargoShip : public Ship
    {

    // Set the member variable for tonnage / capacity

    int capacity;

    public:

        // Default Constructor

        CargoShip() : Ship() {
            capacity = 0;
        }

        //Overloaded constructor for CargoShip, calls parent class

        CargoShip(int tonnage, string name) : Ship() {

            CargoShip::capacity = tonnage;
        }

        // Accessor Function 
        int getCapacity() const {
            return capacity;
        }

        //Mutator Function
        void setCapacity(int tonnage) {
            cout << " Enter max capacity: " << endl;
            cin >> tonnage;
            tonnage = capacity;
        }

        //Overriding Print Function
        virtual void print() const override{
            cout << " Name: " << nameOfShip << endl;
            cout << " Capacity: " << capacity << endl;
        }
    };


    int main()
    {

        // Pointer Array for Ships, listing the 3 ship classes

        Ship *shipArray[3] = { new Ship(), new CruiseShip(), new CargoShip() };


        // For loop to print the data for each of the 3 ships
        for (int i = 0; i < 3; i++) 
        {
            shipArray[i]->print();
            cout << endl;
        }

        // Stores new data using the mutator functions within the class
        shipArray[0]->setShipName("RMS Titanic");
        shipArray[0]->setShipYear(1909); 

        // Pointers to the derived class, stores new data for functions in CruiseShip class
        CruiseShip *csPoint = static_cast<CruiseShip*>(shipArray[1]);
        csPoint->setShipName("HMS Victory");
        csPoint->setMaxPass(850);

        // Pointer to the derived class, stores new data for functions in CargoShip class
        CargoShip *cgPoint = static_cast<CargoShip*>(shipArray[2]);
        cgPoint->setShipName("HMHS Britannic");
        cgPoint->setCapacity(48158);

        //For loop to re-display updated data using base class pointers
        for (int i = 0; i < 3; i++) {
            shipArray[i]->print();
            cout << endl;
        }

        return 0;

    }

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • 2
    `name = nameOfShip;` doesn't make a whole lot of sense in your `setShipName` member. That overwrites the value-just-read from `cin` with whatever value is in the member `nameOfShip` (which is literally nothing). It then discards `name` and nothing in the object changed whatsoever. The same broken logic is repeated for other member(s) in other `set` functions. – WhozCraig Jun 17 '20 at 19:35
  • "virtual void print() const override" - ditch the `virtual`, it's redundant when you also say `override`. – Jesper Juhl Jun 17 '20 at 19:36
  • An extension to a comment by Jesper Juhl up a few comments: [`virtual`](https://en.cppreference.com/w/cpp/language/virtual) is sticky. Once you make a function `virtual`, it stays `virtual` in derived classes. But watch out for [`final`](https://en.cppreference.com/w/cpp/language/final). – user4581301 Jun 17 '20 at 19:40
  • Good job on avoiding [object slicing](https://stackoverflow.com/questions/274626/what-is-object-slicing). From the *display the base class data again* in the problem description I was totally expecting an object slicing question. – user4581301 Jun 17 '20 at 19:43
  • Removing the 'name = nameOfShip' and the others after 'cin' and the 'virtual' doesn't change anything as to how the code works. It still doesn't recognise the inputted data and redisplay it once it has been entered. I do apologise, I am very new to the language – Lydia Hodges Jun 17 '20 at 19:43
  • Removing the `name = nameOfShip;` isn't the solution. You have it backward. You're assigning `nameOfShip` to `name`, then you want `nameOfShip = name;` Just like in regular math, assignment replaces the value on the left with the value on the right. – user4581301 Jun 17 '20 at 19:46
  • But surely the values should be changed using 'cin' then? If the program is taking the console input and assigning it to 'name' then should it not display the inputted data? – Lydia Hodges Jun 17 '20 at 19:49
  • 2
    Just lose the `cin` interface entirely. Those setters should do one thing: set the data provided as an argument to the member. I.e. `nameofShip = name;` for example. If the name is coming from `cin`, great, but that shouldn't be in the setter; it should be in the *caller code* before it ever calls the setter with the value already-read from `cin` (or wherever). Just get it working by losing the caller interface. And fyi, those `static_cast` should be `dynamic_cast` in `main`. – WhozCraig Jun 17 '20 at 19:49

1 Answers1

0

For starters these data members

    int yearBuilt;
    string nameOfShip;

should have the access specifier protected instead of public.

In this constructor

    Ship(int year, string name) {

        Ship::yearBuilt = year;
        Ship::nameOfShip = name;

        year = 0;
        name = "";
    }

the last two statements are redundant and does not make sense. Define the constructor like

    Ship( int year, const string &name ) : yearBuilt( year ), nameOfShip( name )
    {
    }

It will be better to define these getters

    // Accessor Function for the year the ship was built
    int getYearBuilt() const {
        return yearBuilt;
    }
    // Accessor Function for the name of the ship
    string getShipName() const {
        return nameOfShip;
    }

like

    // Accessor Function for the year the ship was built
    const int & getYearBuilt() const {
        return yearBuilt;
    }
    // Accessor Function for the name of the ship
    const string & getShipName() const {
        return nameOfShip;
    }

These setters do not make sense. For example arguments are not used. And moreover you are trying to reassign arguments with data members.

    // Mutator Function to read input and set it to the value of yearBuilt
    void setShipYear(int year) {
        cout << " Enter Year: " << endl;
        cin >> year;
        year = yearBuilt;

        cout << endl; // Spacing between printed data
    }
    // Mutator Function to read input and set it to the value of nameOfShip
    void setShipName(string name) {
        cout << " Enter Name: " << endl;
        cin >> name;
        name = nameOfShip;

        cout << endl; // Spacing between printed data
    }

they should be defined at least like

    // Mutator Function to read input and set it to the value of yearBuilt
    void setShipYear(int year) {
        yearBuilt = year;
    }
    // Mutator Function to read input and set it to the value of nameOfShip
    void setShipName(string name) {
        nameOfShip - name;
    }

This constructor

    //Overloaded Constructor
    CruiseShip(int maximum, int year, string name) : Ship() {
        CruiseShip::passengersMAX = maximum;
    }

does not initialize data members of the base class with the passed arguments. It should be defined like

    //Overloaded Constructor
    CruiseShip(int maximum, int year, const string &name) : Ship(year, name ), passengersMAX( maximum )
    {
    }

This setter

    //Mutator
    void setMaxPass(int maximum) {
        cout << "Enter Passenger Max: " << endl;
        cin >> maximum;
        maximum = passengersMAX;
    }

should be defined like

    //Mutator
    void setMaxPass(int maximum) {
        passengersMAX = maximum;
    }

This constructor

    CargoShip(int tonnage, string name) : Ship() {

        CargoShip::capacity = tonnage;
    }

does not initialize with the argument the base class sub-object. Moreover the data member yearBuilt does not have a corresponding argument.

So at least the constructor should be defined like

    CargoShip(int tonnage, string name) : Ship( 0, name ), capacity( tonnage )
    {
    }

This setter

   //Mutator Function
    void setCapacity(int tonnage) {
        cout << " Enter max capacity: " << endl;
        cin >> tonnage;
        tonnage = capacity;
    }

should be defined like

   //Mutator Function
    void setCapacity(int tonnage) {
        capacity = tonnage;
    }

In these declaration

    CruiseShip *csPoint = static_cast<CruiseShip*>(shipArray[1]);
    //...
    CargoShip *cgPoint = static_cast<CargoShip*>(shipArray[2]);

you should use reinterpret_cast

    CruiseShip *csPoint = reinterpret_cast<CruiseShip*>(shipArray[1]);
    //...
    CargoShip *cgPoint = reinterpret_cast<CargoShip*>(shipArray[2]);
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • `dynamic_cast` would be more appropriate here than `reinterpret_cast`, yes? – user4581301 Jun 17 '20 at 20:13
  • Changing all of that just gives me a loads of new errors that i now have no idea how to fix :/ Like if i change the nameOfShip and YearBuilt to protected, then it's inaccessible and i have no clue how to change that... – Lydia Hodges Jun 17 '20 at 21:24
  • Turns out the only thing preventing it from working as expected was the 'tonnage = capacity' , 'maximum = passengersMax' and the other two were simply the wrong way around. Switching them seems to make the code work as i need it too. I know i use a lot of bad practices so will rectify them now. Thankyou for everyone who has helped me understand where i have gone wrong. You have all helped massively, i cannot thank you enough. – Lydia Hodges Jun 17 '20 at 21:41