Georgi Kyuchukov
Georgi Kyuchukov

Reputation: 21

Strange this-> behaviour

So i have the following class

class Community
{
private:
  char* Name;
  char foundationDate[11];
  Person* founder;
  int maxMembersCount;
  int membersCount;
  Person* members;
  static int communitiesCount;

.....

and i want to implement a copy constructor :

Community::Community(const Community& other)
{
    this->Name = new char[strlen(other.Name)+1];
    strcpy(this->Name,other.Name);
    strcpy(this->foundationDate,other.foundationDate);
    this->founder = other.founder;
    this->maxMembersCount = other.maxMembersCount;
    this->membersCount = other.membersCount;
    this->members = new Person[this->maxMembersCount];
    this->members = other.members;
    communitiesCount++;
}

but this code crashes whenever i say Community A=B; so for me this code seems legit, but when i start debugging there is the message: this-> "unable to read memory". Please help me if you need more code example please let me know.


Community::Community(const char* name , char foundDate[],Person* founder,int maxMembers) {

    this->Name = new char[strlen(name)+1];
    strcpy(this->Name,name);
    strcpy(this->foundationDate,foundDate);
    this->founder = new Person(founder->getName(),founder->getEGN(),founder->getAddress());
    this->maxMembersCount = maxMembers;
    this->membersCount = 2;
    this->members = new Person[this->maxMembersCount];
    communitiesCount++;

}

this is the main constructor of the class which works just fine....

Upvotes: 2

Views: 135

Answers (2)

Aneri
Aneri

Reputation: 1352

First of all, check that other.Name is filled with a pointer to a null-terminated string, that other.foundationDate contains a null-terminated string. That is, you pass good pointers to strlen and strcpy.

If that's true, check that B in the assignment is accessible altogether.

If that's true too, printf everything. And debug where exactly the exception occurs. Or post whole code that is compilable and which reproduces the error.

Also note that here:

this->members = new Person[this->maxMembersCount];
this->members = other.members;

the first assignment does nothing (leaks memory, in fact) while the second double deletes your memory upon object destruction (if you properly delete[] members).

Upvotes: 1

Mark B
Mark B

Reputation: 96271

There are multiple problems here, any of whichi could be part or all of the problem.

  • If Name or foundationDate is not null-terminated on the right-hand side, it will run off and copy bad memory.
  • If founder or members are owned by the object, you will either leak memory if you don't delete them in the destructor, or cause a whole variety of memory-related problems when you shallow-copy and then delete twice, etc.

To fix this, just make your Name and foundationDate std::string, and then make founder and members be owned by value rather than by pointer. If you absolutely have to allocate them on the heap use a smart pointer such as shared_ptr to hold it instead of a bug-prone raw pointer.

Upvotes: 1

Related Questions