-1

I'm trying to initialize one object into other object using copy constructor. I'm puzzled that if I comment out copy constructor it initializes fine but with following code it does not.

class TDateTime : public TData {
public:
  TDateTime() : TData(EDataStateInvalid) {}
  TDateTime(const tm& aTm){};
  TDateTime(int aTm_sec, int aTm_min, int aTm_hour,
  int aTm_mday, int aTm_mon, int aTm_year, int aTm_wday,int aTm_isdst){
      value.tm_sec=aTm_sec;
      value.tm_min=aTm_min;
      value.tm_hour=aTm_hour;
      value.tm_mday=aTm_mday;
      value.tm_mon=aTm_mon;
      value.tm_year=aTm_year;
      value.tm_wday=aTm_wday;
      value.tm_isdst=aTm_isdst; 
  };
  virtual ~TDateTime() {cout<<"Destroying TDateTime ";};

  //! Copy constructor
  TDateTime(const TDateTime& aInstance){};
  //! Copies an instance
  virtual const TDateTime& operator=(const TDateTime& aInstance){return   *this;};
  private:
  tm value;
};



main.cpp
tm=createDateTime(x);
TDateTime     aDateTimeFrom(tm.tm_sec,tm.tm_min,tm.tm_hour,tm.tm_mday,tm.tm_mon,tm.tm_year,tm. tm_wday,0);  
TDateTime aDateTimeTo(aDateTimeFrom);

if I comment out the copy constructor it copies fine. If I remove {} then compiler complains about undefined symbol.

Can you suggest what is wrong here? Based on answer about empty copy constructor does nothing, I comment it out and copy is perfect but I have another problem. If I do

TDateTime aDateTime;
aDateTime=aDateTimeFrom;

aDateTime has all junk values. Any pointers on this?

user437777
  • 1,423
  • 4
  • 17
  • 28
  • 5
    You manually created a copy constructor that does not copy anything. Why are you surprised that it doesn't copy anything? – Mat Jan 10 '16 at 10:52
  • do you mean with this line TDateTime(const TDateTime& aInstance){}; I have not created this, it is already there but I want to copy. I am not sure what is wrong. Why it copies if I comment out the copy constructor? – user437777 Jan 10 '16 at 11:01
  • Is `tm` the one from http://en.cppreference.com/w/cpp/chrono/c/tm? – Christian Hackl Jan 10 '16 at 11:19

3 Answers3

2
  //! Copy constructor
  TDateTime(const TDateTime& aInstance){};

Your copy constructor does nothing. There is no magic involved with user-written copy constructors; if you don't make them do anything, then they will not do anything.

More precisely, a new instance is created, but its members are left uninitialised or are default-initialised.

While we're at it...

//! Copies an instance
  virtual const TDateTime& operator=(const TDateTime& aInstance){return   *this;};

Same problem here. Your copy-assignment operator does not do anything.

By convention, it should also not return a const reference but a non-const one.

It may be worth the mention that your intution seems to be correct, as your goal should indeed be to create classes which don't require any self-written copy constructors or copy-assignment operators because all members know how to correctly copy or copy-assign themselves (like std::string or std::vector or std::shared_ptr). But in that case, rather than defining the member functions with empty implementations, you just don't declare them at all in your code, such that the compiler can handle everything automatically.

And finally, one other thing: A good rule of thumb for C++ classes is that they should either have virtual functions and disable copying (sometimes called "identity classes") or have no virtual functions and allow copying (sometimes called "value classes"). Something like a virtual assignment operator often indicates an overly complicated, hard-to-use and error-prone class design.

Christian Hackl
  • 27,051
  • 3
  • 32
  • 62
1

A compiler generates a copy constructor which performs member-wise copy if the user hasn't declared a copy constructor. If the user does declare a copy-constructor, then the copy constructor does what the user has told it to, in your case - exactly nothing.

TDateTime(const TDateTime& aInstance){ /* empty body*/};
Armen Tsirunyan
  • 130,161
  • 59
  • 324
  • 434
1

Your define a copy constructor which does not do anything.

In your case you don't really need a copy constructor and/or copy assignment operator, the compiler generated versions would be enough (it would be wise to make sure/recheck that tm class can handle the copying).

There is a rule of three, which states that one needs copy constructor and copy assignment operator if one defines a destructor. But it is only a rule of thumb and not an actual necessity. In your case there is no memory management involved and you use the destructor only for logging.

So do not declare a copy constructor at all (your assign operator has the same flaw by the way - it does not copy anything) and let the compiler do the work.

ead
  • 32,758
  • 6
  • 90
  • 153