2

I`m writing string class by myself. And I dont know how to write Сopy constructor. I have such code.

class S { 
         private:
            char *string;
            int l;
         public:   
            S::S(const S &s){
                 string = new char[l+1];
                 memcpy(string,s.string,l+1);
               }
};

Complier didn`t give any errors, but .exe closed by unknown error. I tried it to use in main() function.

S pop("Q6");
S str(pop);

So I`m looking forward to your help.

4 Answers 4

8

In your copy constructor

 string = new char[l+1]

l hasn't been initialized yet to to the length of the string, so it could be any value. You should initialize it (by copying the value from s.l) prior to using it.

1
  • This answers the question, but imho there should be a mentioning of copy-assignment and destruction, and how smart pointers would be a better start. The class as it is is a timebomb. Oct 12, 2012 at 14:12
3

Once you have a copy constructor, you also need to provide an assignment operator and a destructor. This is known as the rule of three.

But better would be to use smart pointers, then you don’t need to pay attention to that, and the rule of three becomes the rule of zero.

You also forgot to initialise l. What’s more, memcpy has no real place in C++ code. Use the standard library algorithms rather than C functions (in this case, std::copy).

1
  • Thank you for your advise. I will try to follow them. And I didnt know about C++ copy function ))
    – Rocketq
    Oct 12, 2012 at 12:30
2

Try this

public:
    S::S(const S &s) : l(s.l)
    {
        string = new char[l+1];
        memcpy(string,s.string,l+1);
    } 
4
  • why not also initialize string in the initialization list? Oct 12, 2012 at 12:21
  • @Rocketq, it is initialization list. See, for example, cprogramming.com/tutorial/initialization-lists-c++.html.
    – nothrow
    Oct 12, 2012 at 12:31
  • @LuchianGrigore - I would if the order of the fields was different (should always use initialization lists in the same order that they are declared. Also might want to consider if you are using the version of new that returns a zero pointer on failure or throws an exception. In production you may wish to consider what needs to in this eventuality.
    – Ed Heal
    Oct 12, 2012 at 12:32
  • Perfectly fits my About me text. Oct 12, 2012 at 14:06
1

The problem is in l+1. l is not initialized. you need to assign it, like, l = s.l; before using.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.