Runtime error when printing std:string.

[Zerpent]

Hi.

I am trying to learn how to use inheritance in C++.

I have a base class: Animal. I also have a sub class : Cat, which inherits Animal. I am trying to call presentYourself(), which is defined inside Animal through the Cat-object, since Cat inherits that function from Animal.
However, I get a runtime error and I think that it has something to do with trying to print that dereferenced string: m_strSpecies, by calling getSpecies().

Main.cpp:

  #include "stdafx.h"
#include <iostream>
#include "Animal.h"
#include "Dog.h"
#include "Cat.h"
#include "Snake.h"


int main()
{
	using namespace std;
	
	Cat cCat1;
	Dog cDog1;
	Snake cSnake1;

	cCat1.presentYourself();


	cin.clear();
	cin.ignore(255,'\n');
	cin.get();
	return 0;
}

Animal.h:

#ifndef ANIMAL_H
#define ANIMAL_H
#include "MyTypes.h"
#include <string>

using namespace std;



class Animal
{
private:
	u8 m_uchLegs;
	string* m_strSpecies;

public:
	Animal():m_uchLegs(0),m_strSpecies(0) {}
	Animal(u8 uchLegs, string* strSpecies ):m_uchLegs(uchLegs),m_strSpecies(strSpecies) {};
	~Animal()
	{
		delete m_strSpecies;
		m_strSpecies = 0;
	}

	u8 getLegs() const;
	const string* getSpecies() const;
	void presentYourself() const;
};

#endif 

Animal.cpp:

#include "stdafx.h"
#include "Animal.h"
#include "MyTypes.h"
#include <string>
#include <iostream>

using namespace std;

u8 Animal::getLegs() const
{
	return m_uchLegs;
}

const string* Animal::getSpecies() const
{
	return m_strSpecies;
}


void Animal::presentYourself() const
{
	cout << "Hi, I am an " << *m_strSpecies << "!" << endl;
	cout << "I have " << m_uchLegs << " legs!" << endl; 
}

Cat.h:

#ifndef CAT_H
#define CAT_H
#include "Animal.h"
#include <string>

class Cat : public Animal
{
private:
public:
	Cat():Animal(4,&string("Cat")){}
};

#endif 

I also want to mention that I am a beginner and my code might therefore have some severe flaws. Please tell me if so.

Thanks for the help.
Best regards, Zerpent.

[MikeyBoy]

In line 10 of Cat.h, you're creating a temporary, nameless object of type string, and passing the address of it to be stored as a data member of Animal.

However, since it's a temporary object, the memory at that address becomes freed up immediately afterwards, so that cCat1.strSpecies is no longer a valid pointer.

[Zerpent]

What would be the best way to send a string to Animal's constructor then?
I realized that I can create a Cat object in the heap and then send the entire object like this:

Cat():Animal(4,new string("Cat")){}

But isn't that inefficient? I only want to send the address, not the entire object.

[MikeyBoy]

Well, if you really want to use pointers like you you're doing now, then dynamically allocate a new string on the heap, and pass that pointer to the constructor, rather than a pointer to a temporary object. You can then either have the Animal object take ownership of the memory, or leave it to the calling code - although be sure you're consistent about how you do that.

But really, I don't know why you're bothering. Just have the Animal class store its own copy of the string, rather than a pointer to one that's been created somewhere else. You can pass the string into the constructor by value, or, if you're really worried about efficiency, as a reference.

[MikeyBoy]

In response to your edit:

I realized that I can create a Cat object in the heap and then send the entire object like this:

Cat():Animal(4,new string("Cat")){}

But isn't that inefficient? I only want to send the address, not the entire object.

That is sending the address only. new returns a pointer to the object that's been created.

[Zerpent]

I rewrote Animal so that it does not have a string pointer, but an actual string.
But I do not know how to pass the new created string from Cat to Animal and then save that inside Animal's member m_strSpecies.

Here is the new code:

Animal.h:

#ifndef ANIMAL_H
#define ANIMAL_H
#include "MyTypes.h"
#include <string>
#include <iostream>

using namespace std;



class Animal
{
private:
	u8 m_uchLegs;
	string m_strSpecies;

public:
	Animal():m_uchLegs(0),m_strSpecies(0) {}
	Animal(u8 uchLegs, string* strSpecies):m_uchLegs(uchLegs),m_strSpecies(strSpecies) {};

	u8 getLegs() const;
	const string& getSpecies() const;
	void presentYourself() const;
};

#endif 

Cat.h:

#ifndef CAT_H
#define CAT_H
#include "Animal.h"
#include <string>

class Cat : public Animal
{
private:
public:
	Cat():Animal(4,new string("Cat")){}
};

#endif 

When I send the address of the new string from Cat's constructor to Animal's constructor, I do not know how to use that address to assign the new string object to Animal's member m_strSpecies.

Thanks for your help.

[Cubbi]

Just don't make pointers to strings in the first place (and while you're at it, remove "using namespace" from header files)

#include <string>
#include <cstdint>
#include <iostream>

class Animal
{
private:
    std::uint8_t m_uchLegs;
    std::string m_strSpecies;

public:
    Animal(std::uint8_t uchLegs = 0, const std::string& strSpecies = "")
        : m_uchLegs(uchLegs), m_strSpecies(strSpecies) {};

    std::uint8_t getLegs() const;
    std::string getSpecies() const { return m_strSpecies; }
    void presentYourself() const;
};

class Cat : public Animal
{
public:
    Cat() : Animal(4, "Cat") { }
};

int main()
{
    Cat c;
    Animal& a = c;
    std::cout << "a's species: " << a.getSpecies() << '\n';
}

online demo: http://ideone.com/LQoKmp

[Zerpent]

Ok, I see :).

I have 2 last questions.

1: Why do some people say that it's bad to have "using namespace std" instead of for example writing std::... like you did now?

2: I have read that you always should have a destructor that deletes/frees the
heap memory when you have dynamically allocated memory. This memory is
the string that I create in my Cat constructor with 'new" operator. I have
learnt that if you had a pointer let's say: Animal* pAnimal = new Cat;, you
free that memory by writing: delete pAnimal;. But now I do not have a pointer
in the class Animal but an actual string. So to free that memory, can I just
reference it and delete it like this instead?: delete &m_strSpecies.

[Disch]

1) Because it defeats the entire point of having a separate namespace.

using namespace std; takes everything in the std namespace (even stuff you don't know about) and dumps it into the global namespace. This increases the risk of name conflicts.

Typing out the std:: prefix is also more verbose and can help with code clarity.



2) You only delete if you new. If you did not new the object, then you must not delete it.

In this case... since you are not newing the string, you should not delete it at all. It will be automatically cleaned up... just as it is automatically allocated.

[Cubbi]

Why do some people say that it's bad to have "using namespace std" instead of for example writing std::... like you did now?

it's not always bad on its own, but it's bad in a header: see C++ Coding Standards by Sutter and Alexandrescu, item 59.

But now I do not have a pointer in the class Animal but an actual string. So to free that memory, can I just reference it and delete it like this instead?:

No, to free the memory occupied by the string, you do nothing: destructors of class members are called automatically.

[Zerpent]

Ok, I will take a closer look at how namespaces work and I will remember to pass strings like you did from now on.

Thanks a lot guys for the help.

Btw, I recognized your name Disch :). You taught me how to write binary files a year ago.