C++ copy constructor

[Robertlzw]

I am staring at the following code:

template <typename T>
class A {
public:
    A(const A &src) { *ptr = *(src.ptr); }
private:
    T *ptr;
}

Is the above code correct?
I do not feel comfortable with it. In case T is char, I usually implement it as follows:

class A {
public:
    A(const A &src) { 
      char *old = ptr;
      ptr = new char[strlen(src.ptr) + 1];
      strcpy(ptr, src.ptr);
      delete old;
    }
private:
    char *ptr;
}

They are fundamentally different!

[Computergeek01]

You're right in that they are different but in your version you can't assign your class to be any more useful then a char, where as the origional would have cast it to a char and anything else. Also your version won't know what to do if I pass it a file stream for example.

It would be better if you took YOUR version of the constructor and turned it into a specilized template.

[Athar]

Whether the first is correct depends on what it's supposed to be used for.
Currently it has little use as it just stores a pointer that cannot be accessed.
Edit: that answer would have applied to ptr = src.ptr; *ptr = *(src.ptr); is definitely wrong.

Also, there's an error in your second constructor: you have to use delete[] instead of delete.

[Galik]

This code...

template <typename T>
class A {
public:
    A(const A &src) { *ptr = *(src.ptr); }
private:
    T *ptr;
}

... doesn't free the memory assigned to ptr.

EDIT:

Oops, my mistake, its a constructor... deallocation is not an issue :)

[tition]

Well, as it is your template *should* cause an immediate run-time Segfault: you never allocate the ptr!

I think the templated version should be written like this:

template <typename T>
class A {
public:
  A(const A &src) 
  { this->ptr = new T;
    *this->ptr = *(src.ptr);   
  }
  ~A(){delete this->ptr; this->ptr=0;}
//I would also add the following constructor
  A(){this->ptr=0;}
private:
    T *ptr;
};

What is the point of this template?
Here is a template I found useful with similar looks to yours.

template <class Object>
class MemorySaving
{
private:
  Object* theValue;
  MemorySaving(const MemorySaving<Object>& other){assert(false);}
public:
  void operator=(const MemorySaving<Object>& other)
  { if (!other.IsZeroPointer()) 
      (this->GetElement())=(other.GetElementConst()); 
    else 
      this->FreeMemory();
  }
  const Object& GetElementConst()const
  { assert(this->theValue!=0); 
    return *this->theValue;
  }
  Object& GetElement()
  { if (this->theValue==0)
      this->theValue=new Object;
    return *(this->theValue);
  }
  bool IsZeroPointer()const{ return this->theValue==0;}
  void FreeMemory()
  { delete this->theValue;
    this->theValue=0;
  }
  MemorySaving(){this->theValue=0;}
  ~MemorySaving(){this->FreeMemory();}
};

[Computergeek01]

Aren't we getting a little off topic with the memory leaks guys? I agree they are important but it's pretty clear, to me at least, that the OP only wanted to know about the proper installation of a template class in case it was a char.

EDIT: I doubt this is even a functional class they would be using in the real world. How could it be?

[Thanz]

Galik n Computergeek01 do u both use gmail or skype???if yea, share ua accounts..

[tition]

Well, I think that it is semantically incorrect inconvenient (bordering with incorrect) to use copy constructors other than, well, for constructing things :). [Edit: Computergeek01 corrected me on this one]

For copying stuff around I always use operator= (overloaded). And of course, I try to be careful with the extremely annoying semantics ambiguity:

Rational tmp=0; //calls the constructor Rational(int)
Rational tmp;
tempRat2=0; //calls the overloaded operator Rational::operator=(int) 

[Computergeek01]

@ Thanz: I'm right here on the form so feel free to comment, PM me or start a thread of your own. I understand the idea that the individual attention may lead to a quicker solution to your questions but I can't even pretend to be the most informed person on this site.

@ tition: But there is a huge difference between those two. A copy constructor would create an new instance of an object where as the operator would copy data to an already existing instance. I guess I don't get what you are trying to say here.

[Robertlzw]

Thanks for the discussion. I should have used "delete [ ] old".

If we look at what the copy ctor for class template does in its implementation, it looks like:

T *ptr;     // unitialized pointer to T
*ptr = *(src.ptr);

It would be the same as we say:

int main() {
   int *p1, *p2;
   p1 = new int(10);
   *p2 = *p1;
}

Is it ugly? Has ptr already been initialized somewhere before the copy ctor is called?


EDIT: did someone notice that my implementation of the copy ctor was not correct (that code was for copy assignment operator)? It should look like:

class A {
public:
    A(const A &src) { 
      ptr = new char[strlen(src.ptr) + 1];
      strcpy(ptr, src.ptr);
    }
private:
    char *ptr;
}

[tition]

@computergeek01:
oops: I messed up copy constructors - just found out that this

#include <iostream>

using namespace std;

class A
{
public:
  A(const A& other){ }
  A(){}

};

int main()
{ A x,y;
  x.A::A(y);
  return 0;
}

doesn't compile. I thought it should compile (although it is clearly a horrible abuse of the language). My bad.