-2

I'm new to cpp and want to learn about list. When I run my program it compiles fine but I don't get the result I'm expecting.

This gives me the output 

Legs: 0 Name:

It should be 

Legs 4 Name: dog

Can any one see the problem?

As in the comment section, I have tried for over an hour and get get this to work, can't see the problem.

Header file

using namespace std;

 #ifndef ANIMAL_H
 #define ANIMAL_H

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

    int _Legs;
    void SetName(string name);
    string GetName();
private:
    string _Name;
};

#endif /* ANIMAL_H */

Animal.cpp

    Animal::Animal() {
    }

    Animal::Animal(const Animal& orig) {
    }

    Animal::~Animal() {
    }

    string Animal::GetName(){return _Name;}

    void Animal::SetName(string name){_Name = name;};

Main file

void List()
{

   std::list<Animal> animal_list;

   Animal temp;

   temp = Animal();
   temp._Legs = 4;
   temp.SetName("Dog");
   animal_list.push_back(temp);


for(std::list<Animal>::iterator list_iter = animal_list.begin(); 
    list_iter != animal_list.end(); list_iter++)
{
    std::cout<< "Legs:" << list_iter->_Legs << " Name: " << list_iter->GetName() << endl;
}


}

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
LPSTR lpCmdLine, int nCmdShow)
{

   List();
   return 0;
}
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
Niklas
  • 1,753
  • 4
  • 16
  • 35
  • 1
    Maybe you should implement that copy-constructor? I mean, actually make it do something, or remove it completely. – Some programmer dude Sep 23 '16 at 16:18
  • 1
    You have an empty copy constructor for `Animal`? So what do you expect actually? Either don't define one at all and use the compiler generated one, or copy the class member variables in the definition. – πάντα ῥεῖ Sep 23 '16 at 16:18
  • 3
    Also (but unrelated to your problem) you should not have names with a leading underscore followed by an upper-case letter. [Those are reserved in all scopes](http://stackoverflow.com/a/228797/440558). – Some programmer dude Sep 23 '16 at 16:20
  • 2
    Moral of the story -- don't code empty copy constructor, assignment operators, etc.,just as stubs or "I will fill this in later" type stuff. Either code the whole thing correctly, or make them `private` and unimplemented, or not write them at all. The compiler will be calling these stubbed out functions with the expectation that you will make copies correctly. – PaulMcKenzie Sep 23 '16 at 16:25
  • Why all the negative points? I had all my code and it was a beginners mistake, a easy problem. But its hard if you dont know about it. – Niklas Sep 23 '16 at 16:42
  • Not a downvoter, but... What you provided was not complete (left out all of the includes), you showed none of the results of your own debugging, and you asked about a problem that would have been very quickly identified by [the debugger that came with your development environment](https://en.wikipedia.org/wiki/Debugger). On the point of the debugger, it is one of the best productivity tools a programmer has at their disposal and the sooner you get familiar with it's use, the sooner you can start reaping the time savings. – user4581301 Sep 23 '16 at 17:46

2 Answers2

4

The reason you have nothing is your copy constructor is incorrect. You make a copy of temp when you add it into the list and it is this copy that you are accessing.

You have

Animal::Animal(const Animal& orig) {
}

Which means when you make a copy, all the members of the class are default initialized and nothing else happens.

The simplest solution is to just get rid of it and let the compiler provided default copy constructor do the copy for you.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
3
Animal::Animal(const Animal& orig) {
}

This is a (empty) copy constructor. Thus when copying an object of your class, e.g. when you ...

animal_list.push_back(temp);

... add it to the list, the copy won't set it members appropriately.

Solution: Just remove that copy constructor (as well as the destructor). See the rule of zero.

Daniel Jour
  • 15,896
  • 2
  • 36
  • 63