Problem with my NULL-terminated string class

I was writting a null terminated string class, and it was going pretty smoothly, but my pushBack function seems to not work, here is some of the code:

Code:

///////////////// STRING.H //////////////////////////
#define MAX_STRING_SIZE 256

class String {
public:
	/* Constructors */
	String();
	String(const char string[]);
	
	/* Public Methods */
	void  setString(const char string[]);
	void  outputString(ostream &os) const;
	
	int   getLength() const;
	
	void  pushBack(const String &s1);
	
	friend ostream& operator << (ostream& os, String s1);
	friend String operator + (String s1, String s2);
	friend bool operator == (String s1, String s2);
	
private:
	char s[MAX_STRING_SIZE];
};

///////////////////// STRING.CPP /////////////////
#include "String.h"

/* Constructors */
String::String() {
	s[0] = '\0';	
}

String::String(const char string[]) {
	setString(string);	
}

/* Public Methods */
void String::setString(const char string[]) {
	int count = 0;
	while(string[count++])
		s[count-1] = string[count-1];
		
	s[count-1] = '\0';
}

void String::outputString(ostream &os) const {
	os << s;	
}

int String::getLength() const {
	int count = 0;
	while(s[count++])
		;
		
	return count-1;
}

void String::pushBack(const String &s1) {
	for(int i = 0; i < s1.getLength(); i++)
		s[getLength()+i] = s1.s[i];
	
	s[getLength()+s1.getLength()] = '\0';
}

ostream& operator << (ostream& os, String s1) {	
	s1.outputString(os);
	return os;
}

/////////////// MAIN.CPP /////////////////////
int main() {
	String string1("String 1!");
	cout << string1 << endl;
	cout << "Length: " << string1.getLength() << endl;
	
	String string2("String 2!");
	cout << string2 << endl;
	cout << "Length: " << string2.getLength() << endl;
	
	string1.pushBack(string2);
	cout << string1 << endl;
	cout << "Length: " << string1.getLength() << endl;
	 
	return 0;	
}

Here is the output:
-------------------
c_olin@bluebox ~/projects/client/Release $ ./client
String 1!
Length: 9
String 2!
Length: 9
String 1!S
Length: 10
------------------

As you can see... pushBack() is giving weird results..

Any ideas? I've been playing around with it for a long time, but cannot seem to figure out why it does this!

Thanks

I have a suspicion it has something to do with the fact that you call getLength inside this loop:

Code:

for(int i = 0; i < s1.getLength(); i++)
		s[getLength()+i] = s1.s[i];

Each time you call it, the length would be different, which probably isn't what you want, except that after the first assignment the last non-garbage character won't be null anymore and getLength will return the incorrect length.

You might want to store the length of your string as a class member, or at least in that function before you loop (it would be somewhat inefficient to call getLength over and over again there anyways, for the loop condition and the index)

Ah... Thanks, that was, infact, the problem. For some reason I never caught that.

Thanks

Grab a pencil and some paper. Draw 15 boxes touching side by side across the top of the paper. Enter what you think string1 looks like into those boxes.

Code:

void String::pushBack(const String &s1) {
	for(int i = 0; i < s1.getLength(); i++)
		s[getLength()+i] = s1.s[i];

What does getLength() return the first time through the loop? What position gets overwritten? Erase what is in that box and write the new value. What does getLength() return the second time through the loop?

Even if calling getLength() worked the way you thought it did, calling it over and over again is inefficient.