Code Help

[aaronWbryant]

I need some coding help. It keeps crashing and is not printing
anything to the screen. I have no idea what to fix. Any help is
greatly appreciated.

Here is my driver:

#include <iostream>
#include <fstream>
#include <cstdlib>
#include "MyString.h"

using namespace std;

int main()
{
MyString fname;
MyString lname("Smith");
MyString mname(lname);
MyString line, paper;
MyString fullName;
ifstream inData;

fname = "Bob";

fname = fname;

cout << "first name: " << fname << endl;

cout <<fname<<" "<<mname<<". "<<lname<<endl;


inData.open("paper.txt");

if(!inData)
{
cerr <<"Error: File NOT open\n";
exit(0);
}

line.getline(inData);

while(!inData.eof())
{
line += "\n";
paper += line;
line.getline(inData);
}

line += "\n";
paper += line;

cout <<"The paper has a length of "<<paper.length()<<".\n";
cout << line;

cout << "\n\n\n";

cout << paper;

cout << "\n\n\n";

//Test the = operator
mname = "A";
fname = fname + " " + mname + ". " + lname;

fullName = fname;

cout << "Fill Name is : "<<fullName<<endl;


if(fname == fullName)
cout<<"But the names remain the same\n";
else
cout<<"Why did you change your name?\n";

fname = "Mary";
if(fname == fullName)
cout<<"But the names remain the same\n";
else
cout<<"Why did you change your name?\n";

for(int index = 0; index < fullName.length(); index++)
cout << fullName[index] << endl;
return 0;
}


Here is my header file:

#ifndef _MYSTRING
#define _MYSTRING
#include <iostream>

using namespace std;

class MyString
{
private:

int size;
int capacity;
char *data;

public:

MyString();
MyString(char *);
MyString (const MyString&);
~MyString();
MyString operator =(const MyString &);
MyString& append (const MyString&);
MyString& erase();
MyString operator + (const MyString&) const;
bool operator == (const MyString&);
bool operator < (const MyString&);
bool operator > (const MyString&);
bool operator <= (const MyString&);
bool operator >= (const MyString&);
bool operator != (const MyString&);
void operator += (const MyString&);
char& operator [] (int);
void getline (istream&);
int length () const;
friend ostream& operator<< (ostream&, MyString&);
void grow();
};



#endif



Here are my functions:



#include "MyString.h"
#include <iostream>
#include <cstdlib>


using namespace std;


MyString :: MyString()
{
size = 0;
capacity = 1;
data = new char[capacity];
data[0] = '\0';
}

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}
strcpy(data, s);
data[size + 1] = '\0';
}

MyString :: MyString (const MyString& s)
{
size = s.size;
capacity = 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

strcpy(data, s.data);
data[size + 1] = '\0';
}

MyString :: ~MyString()
{
delete []data;
}

MyString MyString :: operator =(const MyString& s)
{
delete [] data;

size = s.size;
capacity = 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

strcpy(data, s.data);
data[size + 1] = '\0';
return *this;
}

MyString& MyString :: append (const MyString& s)
{
size += s.size;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size + 1] = '\0';
return *this;
}

MyString& MyString :: erase()
{
size = 0;
data[0] = '\0';
return *this;
}

MyString MyString :: operator + (const MyString& s) const
{
char *addition;
addition = new char[strlen(data) + strlen(s.data)];
strcpy(addition, strcat(data, s.data));
addition[size] = '\0';
return addition;
}

bool MyString :: operator == (const MyString& s)
{
if (strcmp(data, s.data) == 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator < (const MyString& s)
{
if (strcmp(data, s.data) < 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator > (const MyString& s)
{
if (strcmp(data , s.data) > 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator <= (const MyString& s)
{
if (strcmp(data, s.data) <= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator >= (const MyString& s)
{
if (strcmp(data, s.data) >= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator != (const MyString& s)
{
if (!strcmp(data, s.data))
{
return true;
}

else
{
return false;
}
}

void MyString :: operator += (const MyString& s)
{
size += s.size;
capacity = 1;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size] = '\0';
}

char& MyString :: operator [] (int n)
{
return data[n];
}

void MyString :: getline (istream& in)
{
size = 0;
char c;
in.get(c);
while(c != '\n' && in)
{
data[size] = c;
size++;
if (size >= capacity)
{
grow();
}
in.get(c);
}
data[size] = '\0';
}

void MyString :: grow()
{
char *temp;
temp = data;
data = new char[capacity];
strcpy(data, temp);
data[size] = '\0';
delete [] temp;
}

int MyString :: length () const
{
return size;
}

ostream& operator<< (ostream& out, MyString& m)
{
out << m.data << endl;
return out;
}

 

[Christopher]

I need some coding help. It keeps crashing and is not printing
anything to the screen. I have no idea what to fix. Any help is
greatly appreciated.

Here is my driver:

#include <iostream>
#include <fstream>
#include <cstdlib>
#include "MyString.h"

using namespace std;

int main()
{
MyString fname;
MyString lname("Smith");
MyString mname(lname);
MyString line, paper;
MyString fullName;
ifstream inData;

fname = "Bob";

fname = fname;

cout << "first name: " << fname << endl;

cout <<fname<<" "<<mname<<". "<<lname<<endl;

inData.open("paper.txt");

if(!inData)
{
cerr <<"Error: File NOT open\n";
exit(0);
}

line.getline(inData);

while(!inData.eof())
{
line += "\n";
paper += line;
line.getline(inData);
}

line += "\n";
paper += line;

cout <<"The paper has a length of "<<paper.length()<<".\n";
cout << line;

cout << "\n\n\n";

cout << paper;

cout << "\n\n\n";

//Test the = operator
mname = "A";
fname = fname + " " + mname + ". " + lname;

fullName = fname;

cout << "Fill Name is : "<<fullName<<endl;

if(fname == fullName)
cout<<"But the names remain the same\n";
else
cout<<"Why did you change your name?\n";

fname = "Mary";
if(fname == fullName)
cout<<"But the names remain the same\n";
else
cout<<"Why did you change your name?\n";

for(int index = 0; index < fullName.length(); index++)
cout << fullName[index] << endl;
return 0;
}

Here is my header file:

#ifndef _MYSTRING
#define _MYSTRING
#include <iostream>

using namespace std;

class MyString
{
private:

int size;
int capacity;
char *data;

public:

MyString();
MyString(char *);
MyString (const MyString&);
~MyString();
MyString operator =(const MyString &);
MyString& append (const MyString&);
MyString& erase();
MyString operator + (const MyString&) const;
bool operator == (const MyString&);
bool operator < (const MyString&);
bool operator > (const MyString&);
bool operator <= (const MyString&);
bool operator >= (const MyString&);
bool operator != (const MyString&);
void operator += (const MyString&);
char& operator [] (int);
void getline (istream&);
int length () const;
friend ostream& operator<< (ostream&, MyString&);
void grow();

};

#endif

Here are my functions:

#include "MyString.h"
#include <iostream>
#include <cstdlib>

using namespace std;

MyString :: MyString()
{
size = 0;
capacity = 1;
data = new char[capacity];
data[0] = '\0';

}

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}
strcpy(data, s);
data[size + 1] = '\0';

}

MyString :: MyString (const MyString& s)
{
size = s.size;
capacity = 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

strcpy(data, s.data);
data[size + 1] = '\0';

}

MyString :: ~MyString()
{
delete []data;

}

MyString MyString :: operator =(const MyString& s)
{
delete [] data;

size = s.size;
capacity = 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

strcpy(data, s.data);
data[size + 1] = '\0';
return *this;

}

MyString& MyString :: append (const MyString& s)
{
size += s.size;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size + 1] = '\0';
return *this;

}

MyString& MyString :: erase()
{
size = 0;
data[0] = '\0';
return *this;

}

MyString MyString :: operator + (const MyString& s) const
{
char *addition;
addition = new char[strlen(data) + strlen(s.data)];
strcpy(addition, strcat(data, s.data));
addition[size] = '\0';
return addition;

}

bool MyString :: operator == (const MyString& s)
{
if (strcmp(data, s.data) == 0)
{
return true;
}

else
{
return false;
}

}

bool MyString :: operator < (const MyString& s)
{
if (strcmp(data, s.data) < 0)
{
return true;
}

else
{
return false;
}

}

bool MyString :: operator > (const MyString& s)
{
if (strcmp(data , s.data) > 0)
{
return true;
}

else
{
return false;
}

}

bool MyString :: operator <= (const MyString& s)
{
if (strcmp(data, s.data) <= 0)
{
return true;
}

else
{
return false;
}

}

bool MyString :: operator >= (const MyString& s)
{
if (strcmp(data, s.data) >= 0)
{
return true;
}

else
{
return false;
}

}

bool MyString :: operator != (const MyString& s)
{
if (!strcmp(data, s.data))
{
return true;
}

else
{
return false;
}

}

void MyString :: operator += (const MyString& s)
{
size += s.size;
capacity = 1;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size] = '\0';

}

char& MyString :: operator [] (int n)
{
return data[n];

}

void MyString :: getline (istream& in)
{
size = 0;
char c;
in.get(c);
while(c != '\n' && in)
{
data[size] = c;
size++;
if (size >= capacity)
{
grow();
}
in.get(c);
}
data[size] = '\0';

}

void MyString :: grow()
{
char *temp;
temp = data;
data = new char[capacity];
strcpy(data, temp);
data[size] = '\0';
delete [] temp;

}

int MyString :: length () const
{
return size;

}

ostream& operator<< (ostream& out, MyString& m)
{
out << m.data << endl;
return out;

}

I only charge $50 an hr for debugging "it crashes". Let me know if my
rates are acceptable.
Alternatively, narrow down _where_ it crashes, what was expected, what
is happening, and then ask why the two differ. Test one public method
of MyString at a time instead of all at once.

i.e
Test the constructor
Test the grow method
Test the copy constructor
Test the assignment operator
etc.
one by one

 

[Victor Bazarov]

I need some coding help. It keeps crashing and is not printing
anything to the screen. I have no idea what to fix. Any help is
greatly appreciated.

You need to learn to use a "debugger". It's a program that lets
you execute _your_ program statement by statement, also allowing
you to examine the values of all variables and the call stack.
That's the most important tool in the hands of a programmer.

So far I've only found one place where you make a serious mistake:

[..]
MyString :: MyString()
{
size = 0;
capacity = 1;

Does 'capacity' ever change?

data = new char[capacity];
data[0] = '\0';
}

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = 1;

'1'? Really? That's going to be sufficient?

data = new char[capacity];
while (size >= capacity)
{
grow();
}
strcpy(data, s);
data[size + 1] = '\0';
}

MyString :: MyString (const MyString& s)
{
size = s.size;
capacity = 1;

Again, 1?

data = new char[capacity];
while (size >= capacity)
{
grow();

Ok, let's see the 'grow'...

}

strcpy(data, s.data);
data[size + 1] = '\0';
}

MyString :: ~MyString()
{
delete []data;
}

MyString MyString :: operator =(const MyString& s)
{
delete [] data;

size = s.size;
capacity = 1;
One?

data = new char[capacity];
while (size >= capacity)
{
grow();
}

strcpy(data, s.data);
data[size + 1] = '\0';
return *this;
}
[...]

void MyString :: grow()
{
char *temp;
temp = data;
data = new char[capacity];

OK, so you attempt to allocate another character here.
Why? How does the string *grow*? What should happen
to 'capacity'?

strcpy(data, temp);
data[size] = '\0';
delete [] temp;
}

int MyString :: length () const
{
return size;
}

ostream& operator<< (ostream& out, MyString& m)
{
out << m.data << endl;
return out;
}

V

 

[aaronWbryant]

yes I see that now, I have just started started using my debugger and
it crashes when fname = "Bob"; After that statement Bob does get
stored in fname but it crashes.

Here is the updated functions.cpp


#include "MyString.h"
#include <iostream>
#include <cstdlib>


using namespace std;


MyString :: MyString()
{
size = 0;
capacity = size + 1;
data = new char[capacity];
data[0] = '\0';
}

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = size + 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}
strcpy(data, s);
data[size + 1] = '\0';
}

MyString :: MyString (const MyString& s)
{
size = s.size;
capacity = size + 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

strcpy(data, s.data);
data[size] = '\0';
}

MyString :: ~MyString()
{
delete []data;
}

MyString MyString :: operator =(const MyString& s)
{
delete [] data;

size = s.size;
capacity = size + 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

strcpy(data, s.data);
data[size + 1] = '\0';
return *this;
}

MyString& MyString :: append (const MyString& s)
{
size += s.size;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size + 1] = '\0';
return *this;
}

MyString& MyString :: erase()
{
size = 0;
data[0] = '\0';
return *this;
}

MyString MyString :: operator + (const MyString& s) const
{
char *addition;
addition = new char[strlen(data) + strlen(s.data)];
strcpy(addition, strcat(data, s.data));
addition[size] = '\0';
return addition;
}

bool MyString :: operator == (const MyString& s)
{
if (strcmp(data, s.data) == 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator < (const MyString& s)
{
if (strcmp(data, s.data) < 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator > (const MyString& s)
{
if (strcmp(data , s.data) > 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator <= (const MyString& s)
{
if (strcmp(data, s.data) <= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator >= (const MyString& s)
{
if (strcmp(data, s.data) >= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator != (const MyString& s)
{
if (!strcmp(data, s.data))
{
return true;
}

else
{
return false;
}
}

void MyString :: operator += (const MyString& s)
{
size += s.size;
capacity = size + 1;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size] = '\0';
}

char& MyString :: operator [] (int n)
{
return data[n];
}

void MyString :: getline (istream& in)
{
size = 0;
char c;
in.get(c);
while(c != '\n' && in)
{
data[size] = c;
size++;
if (size >= capacity)
{
grow();
}
in.get(c);
}
data[size] = '\0';
}

void MyString :: grow()
{
char *temp;
temp = data;
capacity *= 2;
data = new char[capacity];
strcpy(data, temp);
data[size] = '\0';
delete [] temp;
}

int MyString :: length () const
{
return size;
}

ostream& operator<< (ostream& out, MyString& m)
{
out << m.data << endl;
return out;
}

 

[Alf P. Steinbach]

* (e-mail address removed):

yes I see that now, I have just started started using my debugger and
it crashes when fname = "Bob"; After that statement Bob does get
stored in fname but it crashes.

Here is the updated functions.cpp


#include "MyString.h"
#include <iostream>
#include <cstdlib>


using namespace std;


MyString :: MyString()
{
size = 0;
capacity = size + 1;
data = new char[capacity];
data[0] = '\0';
}

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = size + 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

This loop body will never execute.

strcpy(data, s);
data[size + 1] = '\0';
}

MyString :: MyString (const MyString& s)
{
size = s.size;
capacity = size + 1;
data = new char[capacity];
while (size >= capacity)
{
grow();
}

This loop body will never execute.

strcpy(data, s.data);
data[size] = '\0';
}

MyString :: ~MyString()
{
delete []data;
}

MyString MyString :: operator =(const MyString& s)

Uhuh -- that result type should be a reference.

{
delete [] data;

size = s.size;
capacity = size + 1;
data = new char[capacity];

If this throws you're screwed, having already changed size and capacity.

while (size >= capacity)
{
grow();
}

This loop body will never execute.

strcpy(data, s.data);
data[size + 1] = '\0';
return *this;
}

The default way to implement operator= is

void MyString::swap( MyString& other ) throw()
{
std::swap( size, other.size );
std::swap( capacity, other.capacity );
std::swap( data, other.data );
}

MyString& MyString:perator=( MyString other )
{
swap( other ); return *this;
}

MyString& MyString :: append (const MyString& s)
{
size += s.size;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size + 1] = '\0';
return *this;
}

MyString& MyString :: erase()
{
size = 0;
data[0] = '\0';
return *this;
}

MyString MyString :: operator + (const MyString& s) const
{
char *addition;
addition = new char[strlen(data) + strlen(s.data)];
strcpy(addition, strcat(data, s.data));

Uh oh.

addition[size] = '\0';
return addition;
}

Consider

MyString MyString:perator+( MyString const& s ) const
{
MyString result( *this );

result.append( s );
return result;
}

Since it's much simpler, fixes that bug.

Also, consider making that a freestanding function, in order to support

"blah blah" + someMyString

bool MyString :: operator == (const MyString& s)
{
if (strcmp(data, s.data) == 0)
{
return true;
}

else
{
return false;
}
}

Consider

bool MyString:perator==( MyString const& s ) const
{
return (strcmp( data, s.data ) == 0);
}

Note the essential 'const'.


You want to be able to compare const strings and rvalue strings, don't you?

Also, see above comment about operator+.

bool MyString :: operator < (const MyString& s)
{
if (strcmp(data, s.data) < 0)
{
return true;
}

else
{
return false;
}
}

See above.

bool MyString :: operator > (const MyString& s)
{
if (strcmp(data , s.data) > 0)
{
return true;
}

else
{
return false;
}
}

See above.

bool MyString :: operator <= (const MyString& s)
{
if (strcmp(data, s.data) <= 0)
{
return true;
}

else
{
return false;
}
}

See above.

bool MyString :: operator >= (const MyString& s)
{
if (strcmp(data, s.data) >= 0)
{
return true;
}

else
{
return false;
}
}

See above.

bool MyString :: operator != (const MyString& s)
{
if (!strcmp(data, s.data))
{
return true;
}

else
{
return false;
}
}

See above.

void MyString :: operator += (const MyString& s)
{
size += s.size;
capacity = size + 1;
while(size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size] = '\0';
}

See above.

char& MyString :: operator [] (int n)
{
return data[n];
}

Would be nice with a checking 'at' too.

But more important, don't forget a const version.

You want to be able to index const and rvalue strings, don't you?

void MyString :: getline (istream& in)
{
size = 0;
char c;
in.get(c);
while(c != '\n' && in)
{
data[size] = c;
size++;
if (size >= capacity)
{
grow();
}
in.get(c);
}
data[size] = '\0';
}

Design: i/o has no business being in this class!

void MyString :: grow()
{
char *temp;
temp = data;
capacity *= 2;
data = new char[capacity];
strcpy(data, temp);
data[size] = '\0';
delete [] temp;
}

If you had a constructor taking a capacity, you could do

void MyString::grow()
{
(MyString( 2*capacity ) = *this).swap( *this );
}

int MyString :: length () const
{
return size;
}

ostream& operator<< (ostream& out, MyString& m)
{
out << m.data << endl;
return out;
}

Design: i/o has no business being in this class!


Cheers, & hth.,

- Alf

 

[aaronWbryant]

size is 5 and capacity is 1 but for some reason it only calls grow
once making capacity 2 but the conditions of the while loop are keep
growing until capacity is as large as or larger than size.

 

[mqrk]

yes I see that now, I have just started started using my debugger and
it crashes when fname = "Bob";   After that statement Bob does get
stored in fname but it crashes.

Works for me. What compiler are you using? I'm using gcc.

Also, you probably don't want to be printing an endl with every
string. It makes "cout << fname << mname << lname << endl;" come out
as four lines
Where is says "fill name" you certainly meant "full name"
And the operator+ should set addition[size + s.size] to '\0' (instead
of addition[size])
Otherwise everything works as I would have expected.

Sorry I couldn't be of more help for the crashing business or if I
covered what's already been covered. (Didn't have time to read through
what everyone else said.)

 

[aaronWbryant]

Ok, I have worked a lot of the kinks out but on the driver which is
posted above when fname = fname; for some reason trash gets put into
it even though before this fname is holding Bob but after this call it
holds trash.

Here is my updated functions.cpp

#include "MyString.h"
#include <iostream>
#include <cstdlib>


using namespace std;


MyString :: MyString()
{
size = 0;
capacity = size + 1;
data = new char[capacity];
data[0] = '\0';
}

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = size + 1;

if (size >= capacity)
{
grow();
}

data = new char[capacity];
strcpy(data, s);
data[size] = '\0';
}

MyString :: MyString (const MyString& s)
{
size = s.size;

capacity = size + 1;

if (size > capacity)
{
grow();
}
data = new char[capacity];
strcpy(data, s.data);
data[size] = '\0';
}

MyString :: ~MyString()
{
delete []data;
}

MyString MyString :: operator =(const MyString& s)
{
delete [] data;

size = s.size;

capacity = size + 1;

if (size >= capacity)
{
grow();
}
data = new char[capacity];
strcpy(data, s.data);
data[size] = '\0';
return *this;
}

MyString& MyString :: append (const MyString& s)
{
size += s.size;
capacity = size + 1;
if (size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size + 1] = '\0';
return *this;
}

MyString& MyString :: erase()
{
size = 0;
data[0] = '\0';
return *this;
}

MyString MyString :: operator + (const MyString& s) const
{
MyString result( *this );

result.append( s );
return result;
}

bool MyString :: operator == (const MyString& s)
{
if (strcmp(data, s.data) == 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator < (const MyString& s)
{
if (strcmp(data, s.data) < 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator > (const MyString& s)
{
if (strcmp(data , s.data) > 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator <= (const MyString& s)
{
if (strcmp(data, s.data) <= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator >= (const MyString& s)
{
if (strcmp(data, s.data) >= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator != (const MyString& s)
{
if (!strcmp(data, s.data))
{
return true;
}

else
{
return false;
}
}

void MyString :: operator += (const MyString& s)
{
size += s.size;
capacity = size + 1;
if (size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size] = '\0';
}

char& MyString :: operator [] (int n)
{
return data[n];
}

void MyString :: getline (istream& in)
{
size = 0;
char c;
in.get(c);
while(c != '\n' && in)
{
data[size] = c;
size++;
if (size >= capacity)
{
grow();
}
in.get(c);
}
data[size] = '\0';
}

void MyString :: grow()
{



char *temp;
temp = data;
capacity *= 2;
data = new char[capacity];
strcpy(data, temp);
data[size] = '\0';
delete [] temp;
}

int MyString :: length () const
{
return size;
}

ostream& operator<< (ostream& out, MyString& m)
{
out << m.data;
return out;
}

 

[Thomas J. Gritzan]

Ok, I have worked a lot of the kinks out but on the driver which is
posted above when fname = fname; for some reason trash gets put into
it even though before this fname is holding Bob but after this call it
holds trash.

Here is my updated functions.cpp [...]
MyString MyString :: operator =(const MyString& s)

You didn't read Alf P. Steinbach's comment about this one, did you?

You would want the return type to be a reference.

{
delete [] data;

When you do
str = str;
you delete the string in 'str' and then you want to copy the deleted string
from the right hand side to the left.

If you don't want to use the swap() function, insert this at the top of
this function:

if (this == &s)
return *this;

size = s.size;

capacity = size + 1;

if (size >= capacity)
{
grow();
}

This loop is pointless. size will never be >= than capacity, since capacity
is size + 1.

data = new char[capacity];
strcpy(data, s.data);

strcpy copies the terminating '\0'...

data[size] = '\0';

.... so this assignment (and similar ones) are superfluous.

 

[jason.cipriani]

When you do
str = str;
you delete the string in 'str' and then you want to copy the deleted string
from the right hand side to the left.

If you don't want to use the swap() function, insert this at the top of
this function:

if (this == &s)
return *this;

...... this is off-topic, but I wanted to thank you for pointing that
out. You inadvertently made me realize that I have been failing to
think of the case where somebody does "a = a;" in my code for years
now. I guess it hasn't caused any problems that anybody has noticed
yet, but I'm definitely filing that one away.

Jason

 

[pleatofthepants]

Ok, I am getting to mname = "A"; then it crashes but i am getting all
of the file read but there are some random characters and trash thrown
into the output. I am not sure if my getline function or my += and +
functions are not working correctly.


functions.cpp


#include "MyString.h"
#include <iostream>
#include <cstdlib>


using namespace std;


MyString :: MyString()
{
size = 0;
capacity = size + 1;
data = new char[capacity];
data[0] = '\0';
}

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = size + 1;

if (size >= capacity)
{
grow();
}

data = new char[capacity];
strcpy(data, s);
//data[size] = '\0';

}

MyString :: MyString (const MyString& s)
{
size = s.size;

capacity = size + 1;

if (size > capacity)
{
grow();
}
data = new char[capacity];
strcpy(data, s.data);
//data[size] = '\0';

}

MyString :: ~MyString()
{
delete []data;
}

MyString MyString :: operator =(const MyString& s)
{
if (this == &s)
{
return *this;
}

delete [] data;

size = s.size;
capacity = size + 1;
if (size >= capacity)
{
grow();
}
data = new char[capacity];
strcpy(data, s.data);
//data[size] = '\0';
return *this;
}

MyString& MyString :: append (const MyString& s)
{
size += s.size;
capacity = size + 1;
if (size >= capacity)
{
grow();
}
strcat(data, s.data);
data[size] = '\0';
return *this;
}

MyString& MyString :: erase()
{
size = 0;
data[0] = '\0';
return *this;
}

MyString MyString :: operator + (const MyString& s) const
{
MyString addition;

addition = strcat(data, s.data);
addition[size + s.size] = '\0';
return addition;
}

bool MyString :: operator == (const MyString& s)
{
if (strcmp(data, s.data) == 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator < (const MyString& s)
{
if (strcmp(data, s.data) < 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator > (const MyString& s)
{
if (strcmp(data , s.data) > 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator <= (const MyString& s)
{
if (strcmp(data, s.data) <= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator >= (const MyString& s)
{
if (strcmp(data, s.data) >= 0)
{
return true;
}

else
{
return false;
}
}

bool MyString :: operator != (const MyString& s)
{
if (!strcmp(data, s.data))
{
return true;
}

else
{
return false;
}
}

void MyString :: operator += (const MyString& s)
{
size += s.size;
capacity = size + 1;
if (size >= capacity)
{
grow();
}
strcat(data, s.data);
//data[size + 1] = '\0';
}

char& MyString :: operator [] (int n)
{
return data[n];
}

void MyString :: getline (istream& in)
{
size = 0;
char c;
in.get(c);
while(c != '\n' && in)
{
data[size] = c;
size++;
if (size >= capacity)
{
grow();
}
in.get(c);

}
data[size + 1] = '\0';
}

void MyString :: grow()
{
char *temp;
temp = data;
capacity *= 2;
data = new char[capacity];
strcpy(data, temp);
data[size] = '\0';
delete [] temp;
}

int MyString :: length () const
{
return size;
}

ostream& operator<< (ostream& out, MyString& m)
{
out << m.data;
return out;
}

 

[Alf P. Steinbach]

* pleatofthepants:

Ok, I am getting to mname = "A"; then it crashes but i am getting all
of the file read but there are some random characters and trash thrown
into the output. I am not sure if my getline function or my += and +
functions are not working correctly.

[snip]

MyString :: MyString(char * s)
{
size = strlen(s);
capacity = size + 1;

if (size >= capacity)

Considering that size == capacity - 1 is established, does this 'if' make sense?

{
grow();
}

data = new char[capacity];
strcpy(data, s);
//data[size] = '\0';

}

[snip]

MyString& MyString :: append (const MyString& s)
{
size += s.size;
capacity = size + 1;
if (size >= capacity)

Considering that size == capacity - 1 is established, does this 'if' make sense?

{
grow();
}
strcat(data, s.data);
data[size] = '\0';
return *this;
}

MyString& MyString :: erase()
{
size = 0;
data[0] = '\0';
return *this;
}

MyString MyString :: operator + (const MyString& s) const
{
MyString addition;

addition = strcat(data, s.data);

Previously I wrote "uh oh" about that.

Can you see the bug in the above statement?

addition[size + s.size] = '\0';
return addition;
}

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

Missing 'const', also noted earlier.

Plus some other stuff, also noted earlier.


Cheers, & hth.,

- Alf

 

[James Kanze]

* (e-mail address removed):

[...]

Uhuh -- that result type should be a reference.

{
delete [] data;
size = s.size;
capacity = size + 1;
data = new char[capacity];

If this throws you're screwed, having already changed size and
capacity.

And deleted data! If the new throws, presumably, he'll delete
it again in the destructor, which could be worse than having an
irrelevant size and capacity.

And of course, if he happens to assign an object to itself, he's
also deleted his source data.

This loop body will never execute.

strcpy(data, s.data);
data[size + 1] = '\0';
return *this;
}

The default way to implement operator= is

void MyString::swap( MyString& other ) throw()
{
std::swap( size, other.size );
std::swap( capacity, other.capacity );
std::swap( data, other.data );
}

MyString& MyString:perator=( MyString other )
{
swap( other ); return *this;
}

That's one variant. It's probably preferable to avoid the value
parameter at the interface level---it "reveals" something of the
internal workings. But there are worse sins.

[...]

MyString MyString :: operator + (const MyString& s) const
{
char *addition;
addition = new char[strlen(data) + strlen(s.data)];
strcpy(addition, strcat(data, s.data));

Uh oh.

Yes. We could probably start a small contest: how many things
are wrong with the two preceding lines. (strcat on an
uninitialized array, strcat called before strcpy, not allocating
enough memory...)

 

[James Kanze]

(e-mail address removed) schrieb:

Ok, I have worked a lot of the kinks out but on the driver which is
posted above when fname = fname; for some reason trash gets put into
it even though before this fname is holding Bob but after this call it
holds trash.
Here is my updated functions.cpp [...]
MyString MyString :: operator =(const MyString& s)

You didn't read Alf P. Steinbach's comment about this one, did you?

You would want the return type to be a reference.

{
delete [] data;

When you do
str = str;
you delete the string in 'str' and then you want to copy the
deleted string from the right hand side to the left.

If you don't want to use the swap() function, insert this at
the top of this function:

if (this == &s)
return *this;

Which still doesn't fix the problem if the new[] throws. If you
don't want to use swap, do something else, but whatever you do,
be sure that you've allocated the new memory before freeing the
old.

Also, if you don't want to use swap, be ready to justify in code
review why you've eschewed an idiomatic solution that is known
to work.

 

[Richard Herring]

In message

..... this is off-topic, but I wanted to thank you for pointing that
out. You inadvertently made me realize that I have been failing to
think of the case where somebody does "a = a;" in my code for years
now.

That "somebody" is probably you. Usually it's disguised by syntax like
"*_p = *_q;" and happens somewhere you didn't think of looking, like
inside std::sort.