Why using this way the class is wrong?

Question

I have been told that creating a class like this is wrong without a given explanation and after spending a significant amount of time, I cannot figure out the reason.

class Dog {
public:
    Dog (char const* n) : dog_name(n) {}
    ~Dog() {}

    char const* getDogName() const 
    { 
        return dog_name; 
    }

private:
    char const* dog_name;
};

Probably object lifetime. `dog_name` is pointing to... something. We can't tell what, or what its lifetime is. It might end up being a [dangling pointer](https://stackoverflow.com/questions/17997228/what-is-a-dangling-pointer). — Fred Larson, Mar 05 '18 at 19:33

Fred Larson · Accepted Answer · 2018-03-05T19:50:15.030

Consider the following code using your class:

char *name = new char[5];
strcpy(name, "Fido");
Dog fido(name);
delete name;
std::cout << fido.getDogName() << '\n'; // Oops -- undefined behavior!

The instance fido is still pointing at a char array that has been deleted. This is a dangling pointer, which leads to nasal demons.

Or another problem:

char name[32];
strcpy(name, "Fido");
Dog fido(name);
strcpy(name, "Rover");
Dog rover(name);
std::cout << fido.getDogName() << '\n'; // Why does this print "Rover"?
std::cout << rover.getDogName() << '\n';

Here both instances are pointing at the same buffer. When you changed it, it changed Fido's name to Rover. Oops.

One solution would be to create your own char buffer in the class instead of just a pointer and strcpy the data into it (carefully range checking so you don't overrun the buffer!). Or just avoid all this nonsense and use std::string, which makes a lot more sense.

Why using this way the class is wrong?

1 Answers1