Creating my own String class - getting error message

[Arslan7041]

Im creating my own String class (as an assignment), but I've run into a problem.

All the functions are working properly except for append().

Here is the cpp file (I've shortened it a lot so as not to exceed max limit of post).

 #include "mystring.h"

//**************************************************
// Constructor to initialize the str member        *
// with a string constant.                         *
//**************************************************
MyString::MyString(char *sptr)
{
	len = strlen(sptr);
	str = new char[len + 1];
	strcpy(str, sptr);
}

//*************************************************
// Copy constructor.                              *
//*************************************************

MyString::MyString(MyString &right)
{
	str = new char[right.length() + 1];
	strcpy(str, right.getValue());
	len = right.length();
}

//************************************************
// Overloaded = operator. Called when operand    *
// on the right is another MyString object.      *
// Returns the calling object.                   *
//************************************************

MyString MyString::operator=(MyString &right)
{
	if (len != 0)
	   delete [] str;
	str = new char[right.length() + 1];
	strcpy(str, right.getValue());
	len = right.length();
	return *this;
}

//***********************************************
// Overloaded = operator. Called when operand   *
// on the right is a string.                    *
// Returns the calling object.                  *
//***********************************************

MyString MyString::operator=(const char *right)
{
	if (len != 0)
	   delete [] str;
	len = strlen(right);
	str = new char[len + 1];
	strcpy(str, right);
	return *this;
}

//**************************************************
// Overloaded += operator. Called when operand     *
// on the right is another MyString object.        *
// Concatenates the str member of right to the     *
// str member of the calling object.               *
// Returns the calling object.                     *
//**************************************************

MyString MyString::operator+=(MyString &right)
{
	char *temp = str;

	str = new char[strlen(str) + right.length() + 1];
	strcpy(str, temp);
	strcat(str, right.getValue());
	if (len != 0)
        	delete [] temp;
	len = strlen(str);
	return *this;
}

//****************************************************
// Overloaded += operator. Called when operand       *
// on the right is a string. Concatenates the        *
// string right to the str member of                 *
// the calling object.                               *
// Returns the  the calling object.                  *
//****************************************************

MyString MyString::operator+=(const char *right)
{
	char *temp = str;

	str = new char[strlen(str) + strlen(right) + 1];
	strcpy(str, temp);
	strcat(str, right);
	if (len != 0)
        	delete [] temp;
	return *this;
}

//*********************************************************
// Overloaded == operator.                                *
// Called when the operand on the right is a MyString     *
// object. Returns true if right.str is the same as str.  *
//*********************************************************
bool MyString::operator==(MyString &right)
{
	return !strcmp(str, right.getValue());
}
 //*****************************************************
// Overloaded == operator.                             *
// Called when the operand on the right is a string.   *
// Returns true if right is the same as str.           *
//******************************************************

bool MyString::operator==(const char *right)
{
	return !strcmp(str, right);
}

void MyString::indexError() const
{
    cout << "Error: Index out of bounds." << endl;
    exit(0);
}

// Overloaded [] operator.
char& MyString::operator[](int index) const
{
    // if index out of bounds, print error message
    if(index < 0 || index > len-1)
    {
        indexError();
    }

    else return *(str + index);

}

char& MyString::at(int index) const
{
        // if index out of bounds, print error message
    if(index < 0 || index > len-1)
    {
        indexError();
    }

    else return *(str + index);
}

MyString MyString::append(const char* str2)
{
    return str += str2; 
}

MyString MyString::append(MyString& str2)
{
   return str += str2.str;
}

I have two append functions: One for when the right hand side is a string literal, and the other for when the right hand side is a MyString Object.

I made the function simply by calling the += operator which I've already overloaded to concatenate two strings and return the result.

I thought this would work, but Im getting this error message:

"invalid operand of types char* and const char* to binary 'operator+' ".

Im not sure how to fix this. What am I doing wrong here?

[cire]

Im not sure how to fix this. What am I doing wrong here?

You're trying to add a pointer to a pointer and that is an operation that makes no sense. You didn't do it in the implementation of operator+=, did you? ;)

MyString MyString::append(const char*s)
{
    return *this += s;
}

Likewise with the other. This has less surprising behavior if append doesn't return a new instance of Mystring, but a reference to the existing one append was called on. Of course, you'd also have to change operator+= to also do the expected thing.

[Arslan7041]

I knew it was something obvious.

But im still getting an error message, a different one this time.

Here are my append functions:

MyString MyString::append(const char* str2)
{
    return *this += str2;
}

MyString MyString::append(MyString& str2)
{
   return *this += str2.str;
}

Here is the error message:

"no matching function for call to 'MyString::MyString(MyString)'

What?

[cire]

You're trying to initialize a Mystring object with a temporary, but since your copy constructor is not const correct that isn't possible. Either make it const correct or change the function(s) so there is no temporary involved:

MyString myString::append(const char* s)
{
    MyString result(*this) ;
    result += s;
    return result;
}

[Arslan7041]

Okay, im getting no errors this time and its compiling, but the function still seems to not be working.

Im trying to use it in main, but the result is not correct:

#include <iostream>
#include "mystring.h"

using std::cout;
using std::cin;
using std::endl;

int main()
{
    MyString firstName("First Name");
    MyString lastName(" Last Name");

    firstName.append(lastName);

    cout << firstName << endl;

    return 0;
}

First Name

The expected output was

First Name Last Name

Note that I already have the insertion operator overloaded. I just didn't show it in the OP so as to stay within the character limit of the post.

[cire]

Thought I made an edit to that post. The code I supplied is wrong in that it doesn't affect the calling object, although that's an easy enough fix. Perform the operation on *this before creating result.

I was confused by your unconventional return type, which makes constructs such as:

firstName.append(middleInitial).append(lastName);

give surprising results.

[Arslan7041]

Ah, there we go.

But it also works if we remove 'result' out of the picture altogether. I simply performed the operation on *this and then returned it. It worked fine.

So whats the point in creating result?

[andywestken]

Are you using Microsoft Visual C++?

The behaviour you mention is non-standard -- building with warning level 4 (compiler option /W4) you should get:

warning C4239: nonstandard extension used : 'argument' : conversion from 'MyString' to 'MyString &'
1>          A non-const reference may only be bound to an lvalue; copy constructor takes a reference to non-const

If you alter your project settings to disable Microsoft specific extensions (Project Properties > C/C++ / Language > Disable Language Extensions = Yes) then that code will not compile.

cire's change is required to make your code build with an ANSI strict compiler.

BUT append() should really return MyString& (as should operator= and operator+=)

Andy

[andywestken]

More observations:

1. you need to complete your const correctness

e.g.

MyString::MyString(const char *sptr)

bool MyString::operator==(const MyString &right) const

and esp. replace

char& MyString::operator[](int index) const

with

char MyString::operator[](int index) const

(could use a const char& returns, but that seems a bit heavy for a char)

and

char& MyString::operator[](int index)

2. you need to fix your assignment operator so it can hande self-assignment.

Andy

PS Calling exit() when an invalid index is used seems a bit heavy. If you are not familiar with exceptions, how about:

char& MyString::indexError() const
{
    static char ch = '?';
    cout << "Error: Index out of bounds." << endl;
    return ch;
}

// Overloaded [] operator.
char MyString::operator[](int index) const
{
    // if index out of bounds, print error message
    if(index < 0 || index > len-1)
    {
        return indexError();
    }
    return *(str + index);
}

// Overloaded [] operator.
char& MyString::operator[](int index)
{
    // if index out of bounds, print error message
    if(index < 0 || index > len-1)
    {
        return indexError();
    }
    return *(str + index);
}

[dhayden]

A few other things to consider changing:

Your assignment operator won't work it you assign to youself:

MyString s("hello");
s = s; // won't work
s = s.str; // won't work. 

You're storing the NULL-terminated string but you also have code that says if (len != 0) delete str; It seems to me that if you're storing the null terminated string then str should ALWAYS be present and therefore should always be deleted in those cases.

Since you have the length of the strings, it would be faster you memcpy when copying strings around.