What's wrong with the following class?

[ylxin1993]

class BadClass {
private:
	int len;
	char* p;
public:
	CBadClass(const char* str) : p(str), len(strlen(p)) {}    // bad constructor
	CBadClass() {}
}

I saw in many books the constructor should use the dynamic memory method, but the method above also works fine in simple cases, what's the bad things about the constructor above?

The answer said:

The argument to the constructor could be a string allocated on the heap that could be destroyed externally. This would leave the object with an invalid pointer.

[Fransje]

Say the user does this:

char** string = new char* ( "Hello, World!" );
BadClass badClass( *string );
delete string;

then badClass.p points to de-allocated memory, which will lead to a crash or unexpected behaviour.

[o3hC5Di1]

Hi there,

The answer means the following:

//allocate some memory on the heap
char* cstring = new char();
*cstring = 'a';

//create an object of your class, so far so good.
BadClass b(cstring, 1);

//free the memory, it will not be accessible any longer
delete cstring;
cstring = nullptr;

//the object still holds a pointer to the memory though:
std::cout << *(b.p);  //segfault, can't dereference a null-pointer => program crashes. 

So in short: having pointers within your class point to memory which was allocated outside of the class, i.e. of which the class is not the owner, is very risky, because the class has no way of knowing the pointer points to valid memory or not. If the object is responsible itself for allocation/deallocation, it knows exactly at all times what the state of that memory is.

On a sidenote, you should look into C++11's smart pointers (std::unique_ptr and std::shared_ptr) as a safe way to handle pointers.

Hope that helps.

All the best,
NwN

[toum]

The problem is that if anything happens to the real data, all the BadClass that share its pointer will be affected.
That means if any of them modifies the content of the string, it will be modified for all of them.
And if for some reason the data of the original string gets destroyed, any access through any of the objects that share its pointer will cause memory corruption.

[ylxin1993]

Is there any other better thing using dynamic memory allocation?

=======
plus, what's the usage of the

new char*("Hello, World!");

, I only know the

new char[length]

format.

[o3hC5Di1]

Hi there,

In the case of text-strings, use std::string instead of char-arrays, that will save you a lot of trouble.
In the case of dynamic memory allocation on the whole, you should use C++11 smart pointers (std::unique_ptr and std::shared_ptr).

new char*("Hello, World!"); Will create a new char array of the length of "Hello, World!" and initialise it at the same time. It's basically an easier way to allocate it and to some people is more readable because it's clear from the start which value the variable has.

All the best,
NwN