views:

117

answers:

4

Hey guys. I'm trying to write my own C++ String class for educational and need purposes.
The first thing is that I don't know that much about operators and that's why I want to learn them. I started writing my class but when I run it it blocks the program but does not do any crash.
Take a look at the following code please before reading further:

class CString
{
private:
  char* cstr;
public:
  CString();
  CString(char* str);
  CString(CString& str);
  ~CString();

  operator char*();
  operator const char*();
  CString operator+(const CString& q)const;
     CString operator=(const CString& q);
};

First of all I'm not so sure I declared everything right. I tried googleing about it but all the tutorials about overloading explain the basic ideea which is very simple but lack to explain how and when each thing is called. For instance in my = operator the program calls CString(CString& str); but I have no ideea why.
I have also attached the cpp file below:

CString::CString()
{
 cstr=0;
}
CString::CString(char *str)
{
 cstr=new char[strlen(str)];
 strcpy(cstr,str);
}
CString::CString(CString& q)
{
 if(this==&q)
  return;
 cstr = new char[strlen(q.cstr)+1];
 strcpy(cstr,q.cstr);
}
CString::~CString()
{
 if(cstr)
  delete[] cstr;
}
CString::operator char*()
{
 return cstr;
}
CString::operator const char* ()
{
 return cstr;
}
CString CString::operator +(const CString &q) const
{
 CString s;
 s.cstr = new char[strlen(cstr)+strlen(q.cstr)+1];
 strcpy(s.cstr,cstr);
 strcat(s.cstr,q.cstr);
 return s;
}
CString CString::operator =(const CString &q)
{
 if(this!=&q)
 {
  if(cstr)
   delete[] cstr;
  cstr = new char[strlen(q.cstr)+1];
  strcpy(cstr,q.cstr);
 }
 return *this;
}

For testing I used a code just as simple as this
CString a = CString("Hello") + CString(" World");
printf(a);
I tried debugging it but at a point I get lost. First it calls the constructor 2 times for "hello" and for " world". Then it get's in the + operator which is fine. Then it calls the constructor for the empty string. After that it get's into "CString(CString& str)" and now I'm lost. Why is this happening? After this I noticed my string containing "Hello World" is in the destructor (a few times in a row). Again I'm very puzzeled. After converting again from char* to Cstring and back and forth it stops. It never get's into the = operator but neither does it go further. printf(a) is never reached.
I use VisualStudio 2010 for this but it's basically just standard c++ code and thus I don't think it should make that much of a difference

+3  A: 

The line:

cstr=new char[strlen(str)];

should be:

cstr=new char[strlen(str) + 1];

Also, the test for self-assignment does not make sense in the copy constructor - you are creating a new object - it cannot possibly have the same address as any existing object. And the copy constructor should take a const reference as a parameter,

If in your code, you were expecting the assignment operator to be used, you would be expectining wrong. This code:

CString a = CString("Hello") + CString(" World");

is essentially the same as:

CString a( CString("Hello") + CString(" World") );

which is copy construction, not assignment. The temporary CString "Hello world" will be destroyed (invoking the destructor) after a has been constructed.

Basically, it sounds as if your code is working more or less as expected.

anon
+1 The check for self-assignment was featured in GotW #11 http://www.gotw.ca/gotw/011.htm If I remember correctly the latest version of the book shows how you can actually make the test evaluate to true though (I don't remember exactly how but I'm guessing some sort of placement new) :)
Andreas Brinck
Yeah - it was intended to suggest that doing such was laughable. There's no reason to perform such a check in the copy constructor, it is impossible.
DeadMG
Yes you are right. The test for self assigment doesn't make sense in the constructor since it can't exists before creating it. And thank you for pointing out about the +1 thing.
Sanctus2099
i liked the code joke, +1 :-)
Anders K.
+1  A: 

Here's what's going on:

  1. The constructor is indeed called twice. Once for "hello" and once for " world". The order is undefined.
  2. The CString::operator + is called on the 1st CString ("hello"), passing the second CString (" world") as it's argument. The return value of CString::operator + is a new CString
  3. Since you're assigning in initialization, i.e: CString a = [CString result of operator +], c++ will just call you're copy constructor. Hence the call to CString(CString& ) that you're seeing in the debugger.

Now, that's 4 total objects just created, one for each string literal ("hello", and " world"), one for the concatenation of those (the result of the CString::operator + call, and one to hold the result (CString a = ...). Each one of those temporary objects will have it's destructor called.

As for why you're not getting the printf, I have no idea. I just copy pasted your code in this file:

#include <cstdio>
#include <cstring>

[your code]

int main(int argc,char* argv[]) {
  CString a = CString("hello") + CString(" world");
  printf(a);
}

And when I ran the resulting executable, I got hello world as the output. This is on Ubuntu with g++ 4.4. Not exactly sure why under the VS debugger it's not printing anything.

Bryan Ross
I tought of the same thing but apparently I had to make operator const char* a const overload
Sanctus2099
+1  A: 

Don't use strlen, store your own string length. The string should not be depended on to have a null terminator. It's ok to use such if you're passed in a random const char*, but for internal operations, you should use the size.

Also, you forgot to make your operator const char* a const overload.

DeadMG
Thank you very much. That last bit you told me about fixed it.
Sanctus2099
A: 

A few mistakes you made:

1. Copy constructor signature is wrong. It must be:

CString(const CString& q)

2. op= signature is wrong. It must be:

CString& operator=(const CString& q)

Btw., that was also the reason that the copy constructor was called. You did a return *thisin the end which copied the object (with your op= signature).

3. You allow CString instances with cstr == NULL (your default constructor will result in such an instance). Though, in almost all functions (copy constructor, operator +, operator =) you don't handle that case well (q.cstr == NULL).

Maybe the easiest and safest way would be to just disallow that case and change your default constructor to:

CString::CString()
{
   cstr = new char[1];
   cstr[0] = 0;
}
Albert
Thank you for your advice. I changed the signatures. The thing is that in different places I see different sigantures so that's why I'm not sure which one is the good or "standard" one.And I see the point in making the default constructor as null only terminated string.
Sanctus2099