0

I am new to C++ as I made the switch from Java/C#. Can somebody explain why my code doesn't work like I think it should. I have a simple hierarchy of Animal class which is then inherited by Dog and Cat. The only difference between the classes is their virtual method toString() /which obviously returns a string based on which of the classes it is called on/. Okay so I am inputting information and creating the classes with cin and pushing them into a vector of Animals. However when I tried to call their toString() I got the result from the base toString() and not the overriden one. Here is the code at this point:

#include <iostream>
#include <vector>
#include "animal.h"
#include "cat.h"
#include "dog.h"

using namespace std;

int main()
{
    vector<Animal> animals;

    string type;
    string name;
    int age;
    int weight;

    while(cin >> type){
        cin >> name >> age >> weight;
        if(type == "dog"){
            animals.push_back(Dog(name, age, weight);
        }

        else {
            animals.push_back(Cat(name, age, weight);
        }
    }

    for(vector<Animal>::iterator iter = animals.begin(); iter != animals.end();
        iter++){
            cout << iter -> toString() << endl;
        }

    return 0;
}

But then after I did some googling I found a suggestion that I should use pointers because of something called object slicing. So then my code turned into this:

#include <iostream>
#include <vector>
#include "animal.h"
#include "cat.h"
#include "dog.h"

using namespace std;

int main()
{
    vector<Animal*> animals;

    string type;
    string name;
    int age;
    int weight;

    while(cin >> type){
        cin >> name >> age >> weight;
        if(type == "dog"){
            Dog tempDog(name, age, weight);
            animals.push_back(&tempDog);
        }

        else {
            Cat tempCat(name, age, weight);
            animals.push_back(&tempCat);
        }
    }

    for(vector<Animal*>::iterator iter = animals.begin(); iter != animals.end();
        iter++){
            cout << iter -> toString() << endl;
        }

    return 0;
}

And now I am getting a compiler error suggesting I should use '->';

Also a side question while I am here I would like to ask. Is there a way of overriding a virtual method from the .cpp file and not the header file where the class is defined. I am recently getting into the oop in c++ and to my idea is that in the header file I just define prototypes of the members of the class and I do the implementation in a different .cpp file.

randominstanceOfLivingThing
  • 16,873
  • 13
  • 49
  • 72
  • It's https://en.wikipedia.org/wiki/Object_slicing. Here's the answer: http://stackoverflow.com/questions/274626/what-is-object-slicing – fukanchik Apr 20 '16 at 16:12
  • Oh, and please read this one too: http://stackoverflow.com/questions/408670/stack-static-and-heap-in-c it's not a good idea to store address of heap objects – fukanchik Apr 20 '16 at 16:15
  • Do not store pointer of temporary object (what you are doing with your tempDog and tempCat). You should create them dynamically. – zoska Apr 20 '16 at 16:17
  • Instead of `toString()` you should override `operator<<()` for your class. – Rob K Apr 20 '16 at 18:46

1 Answers1

0
cout << iter -> toString() << endl;

Is attempting to call a member function of the type of *iter. Since *iter is an Animal* it does not have any member functions. What you need to do is get the value of the iterator and then call its member function with -> like

cout << (*iter)->toString() << endl;

Also note that if you have access to C++11 or higher you can use a ranged based for loop like

for (auto& e : animals)
    cout << e->toString() << "\n";

I also changed to "\n" instead of endl as typically you do not need the call to flush so you should only do that when you know you need to.


You also have undefined behavior in your code.

if(type == "dog"){
    Dog tempDog(name, age, weight);
    animals.push_back(&tempDog);
}

Is going to add a pointer to a automatic object into the vector. When you leave the if block that automatic object get automatically destroyed. After it is destroyed you now have a pointer to an object that no longer exist. The quick fix is to use new to dynamically allocate the object like

if(type == "dog"){
    Dog* tempDog = new Dog(name, age, weight);
    animals.push_back(tempDog);
}

Now the pointer in the vector will still be valid. Unfortunately now you need to remember to delete all of those pointers when you are done with them. Instead of having to do manual memory management you can use a smart pointer like a std::unique_ptr or std::shared_ptr which will manage the memory for you.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402