Why does not this main test program work

[Tony Johansson]

Hello!!

I have done some operator overloading but my main testprogram doesn't work
well.

Have you any idea which of my methods are wrong?

#include <iostream>
#include <string>
using namespace std;

class String
{
friend ostream& operator<<(ostream& os, const String& s);
public:
/*explicit*/ String(const char* s);
String(const String& s);
~String() { if(buffer) delete[] buffer; buffer = 0; }
String& operator=(const String& s);
int operator==(const String& s) const; // bool
int operator!=(const String& s) const; // bool
String operator+(const String& s);
String& operator+=(const String& s);

private:
char* buffer;
int size;
};

ostream& operator<<(ostream& os, const String& s)
{
os << s.buffer;
return os;
}

String::String(const char* s)
{
size = strlen(s);
buffer = new char[size+1];
strcpy(buffer, s);
}

String::String(const String& s)
{
size = s.size;
buffer = new char[size+1];
strcpy(buffer, s.buffer);
}

String& String:perator=(const String& s)
{
if (buffer)
delete [] buffer;
buffer = new char[strlen(s.buffer)+1];
strcpy(buffer, s.buffer);
return *this;
}

int String:perator==(const String& s) const
{
if (strcmp(buffer, s.buffer) == 0)
return 1;
else
return 0;
}
int String:perator!=(const String& s) const
{
if (strcmp(buffer, s.buffer))
return 1;
else
return 0;
}

String String:perator+(const String& s)
{
char* str = new char[strlen(buffer) + strlen(s.buffer) + 2];
String local(str);
strcpy(local.buffer, "");
strcat(local.buffer, buffer);
strcat(local.buffer, s.buffer);
return local;
}

String& String:perator+=(const String& s)
{
char* str = new char[strlen(buffer) + strlen(s.buffer) + 1];
String local(str);
strcpy(local.buffer, "");
strcat(local.buffer, buffer);
strcat(local.buffer, s.buffer);
delete [] buffer;
buffer = local.buffer;
size = local.size;
return *this;
}

int main()
{
String s1("Hej");
String s2(" du glade");
String s3(" ta en spade");
String s4(s1 + s2 + s3);
String s5("Test ");
String s6(" av program");
s5 += s6;
cout << s1 << endl << s2 << endl << s3 << endl
<< s4 << endl << s5 << endl << s6 << endl;

return 0;
}

Many thanks

//Tony

 

[mlimber]

Hello!!

I have done some operator overloading but my main testprogram doesn't work
well.

[snip]

In what way does it not work well? Does it compile? Does it run without
crashing? Does it give incorrect results? Etc.

Cheers! --M

 

[Jonathan Mcdougall]

Hello!!

I have done some operator overloading but my main testprogram doesn't work
well.

I assume this is only an exercise and you are aware of std::string.

Have you any idea which of my methods are wrong?

#include <iostream>
#include <string>
using namespace std;
http://www.parashift.com/c++-faq-lite/coding-standards.html#faq-27.5

class String
{
friend ostream& operator<<(ostream& os, const String& s);
public:
/*explicit*/ String(const char* s);
String(const String& s);
~String() { if(buffer) delete[] buffer; buffer = 0; }

delete handles null pointer automatically.

String& operator=(const String& s);
int operator==(const String& s) const; // bool
int operator!=(const String& s) const; // bool

well.. you *are* aware that there is a bool type in C++, aren't you?

String operator+(const String& s);
String& operator+=(const String& s);

private:
char* buffer;
int size;
};

ostream& operator<<(ostream& os, const String& s)
{
os << s.buffer;
return os;
}

String::String(const char* s)

Take the habit of initializing objects.

: size(0), buffer(0)

{
size = strlen(s);
buffer = new char[size+1];
strcpy(buffer, s);
}

String::String(const String& s)
{
size = s.size;
buffer = new char[size+1];
strcpy(buffer, s.buffer);
}

String& String:perator=(const String& s)
{
if (buffer)
delete [] buffer;
buffer = new char[strlen(s.buffer)+1];

why not use s.size?

strcpy(buffer, s.buffer);
return *this;
}

int String:perator==(const String& s) const
{
if (strcmp(buffer, s.buffer) == 0)
return 1;
else
return 0;
}

this should be

bool String:perator==(const String& s) const
{
if (strcmp(buffer, s.buffer) == 0)
return true;
else
return false;
}

int String:perator!=(const String& s) const
{
if (strcmp(buffer, s.buffer))
return 1;
else
return 0;
}
Idem.

String String:perator+(const String& s)
{
char* str = new char[strlen(buffer) + strlen(s.buffer) + 2];
String local(str);

Watch out! str contains garbage here. this won't work! You should
construct the string in str *before* creating local.

strcpy(local.buffer, "");
strcat(local.buffer, buffer);
strcat(local.buffer, s.buffer);
return local;
}

String& String:perator+=(const String& s)
{
char* str = new char[strlen(buffer) + strlen(s.buffer) + 1];

Same here.

String local(str);
strcpy(local.buffer, "");
strcat(local.buffer, buffer);
strcat(local.buffer, s.buffer);
delete [] buffer;
buffer = local.buffer;
size = local.size;
return *this;
}

int main()
{
String s1("Hej");
String s2(" du glade");
String s3(" ta en spade");
String s4(s1 + s2 + s3);
String s5("Test ");
String s6(" av program");
s5 += s6;
cout << s1 << endl << s2 << endl << s3 << endl
<< s4 << endl << s5 << endl << s6 << endl;

return 0;
}

I didn't test it, but if it still doesn't work, be more explicit than
"my main testprogram doesn't work well" and we'll be able to help you
more.


Jonathan

 

[Heinz Ozwirk]

Hello!!

I have done some operator overloading but my main testprogram doesn't work
well.

It doesn't work well? It doesn't work.

Have you any idea which of my methods are wrong?

#include <iostream>
#include <string>
using namespace std;

class String
{
friend ostream& operator<<(ostream& os, const String& s);
public:
/*explicit*/ String(const char* s);
String(const String& s);
~String() { if(buffer) delete[] buffer; buffer = 0; }
String& operator=(const String& s);
int operator==(const String& s) const; // bool
int operator!=(const String& s) const; // bool

Why do these operators return an int? Why not a bool?

String operator+(const String& s);
String& operator+=(const String& s);

private:
char* buffer;
int size;
};

ostream& operator<<(ostream& os, const String& s)
{
os << s.buffer;
return os;
}

String::String(const char* s)
{
size = strlen(s);
buffer = new char[size+1];
strcpy(buffer, s);
}

String::String(const String& s)
{
size = s.size;
buffer = new char[size+1];
strcpy(buffer, s.buffer);
}

String& String:perator=(const String& s)
{
if (buffer)
delete [] buffer;
buffer = new char[strlen(s.buffer)+1];
strcpy(buffer, s.buffer);
return *this;
}

What will happen if you try to assign a string variable to itself? Like

String s("Some String");
s = s;

That might look stupid, but it could happen when a function receives to
string parameters and the caller passes the same variable twice. Also, you
do not keep track of the size of the string. If you don't need its size,
don't add a member to hold it. And if you add such a member, take care that
it holds the proper value. Try something like

char* str = new char[s.size + 1];
strcpy(str, s.buffer);
delete[] buffer;
buffer = str;
size = s.size;
return *this;

int String:perator==(const String& s) const
{
if (strcmp(buffer, s.buffer) == 0)
return 1;
else
return 0;
}

This function's body could also be written as

return strcmp(buffer, s.buffer) == 0;

int String:perator!=(const String& s) const
{
if (strcmp(buffer, s.buffer))

strcmp does not return a boolean value. So don't omit the comparison. After
all, in the function above, you didn't wite !strcmp(...) either.

return 1;
else
return 0;
}

String String:perator+(const String& s)
{
char* str = new char[strlen(buffer) + strlen(s.buffer) + 2];

Why do you want to append two extra characters?

String local(str);

Now you try to initialize a local String variable with something that has
not been initialized. You cannot expect the new'ed memory to contain a
0-terminated string. And what are you trying to do? String's constructor
makes its private copy of the specified string, a string to which no value
has been assigned. You have allocated memory for some string, but you never
initialized that memory.

strcpy(local.buffer, "");
strcat(local.buffer, buffer);
strcat(local.buffer, s.buffer);
return local;
}

You should rearrange you code a little bit:

char* str = new char[strlen(buffer) + strlen(s.buffer) + 1];
strcpy(str, buffer);
strcat(str, s.buffer);
String local(str);
delete[] str;
return local;

But it would be much better to use operator+= to implement operator+:

String local(*this);
local += s;
return local;

String& String:perator+=(const String& s)
{
char* str = new char[strlen(buffer) + strlen(s.buffer) + 1];
String local(str);
strcpy(local.buffer, "");
strcat(local.buffer, buffer);
strcat(local.buffer, s.buffer);
delete [] buffer;
buffer = local.buffer;
size = local.size;
return *this;
}

Here you have similiar problems as in your implementation of operator+. Try

char* str = new char[size + s.size + 1];
strcpy(str, buffer);
strcpy(str + size, s.buffer);
delete[] buffer;
buffer = str;
size += s.size;
return *this;

int main()
{
String s1("Hej");
String s2(" du glade");
String s3(" ta en spade");
String s4(s1 + s2 + s3);
String s5("Test ");
String s6(" av program");
s5 += s6;
cout << s1 << endl << s2 << endl << s3 << endl
<< s4 << endl << s5 << endl << s6 << endl;

return 0;
}

Many thanks

//Tony

HTH
Heinz

 

[David Harmon]

On Fri, 30 Dec 2005 21:39:36 GMT in comp.lang.c++, "Tony Johansson"

String String:perator+(const String& s)
{
char* str = new char[strlen(buffer) + strlen(s.buffer) + 2];
String local(str);

Why do you think it doesn't work? How do the results differ from
your expectations?

Among several mistakes, the above appears to be the most immediate
problem. You allocate some chars with new, which are uninitialized,
then you construct a String that is looking for the end of those
uninitialized chars. Goofyness follows.

Several of your methods fail to set this->size where required.

A proper nonmember operator+() is more like:

String operator+(const String& left, const String& right)
{
String result(left);
result += right;
return result;
}

Of course that depends on a proper operator+=(), copy constructor,
etc.