0

I have homework that I need to make a custom string class using C type strings. Most of it seems okay but I am getting a runtime error as soon as I test it and it isn't performing all my tests like it should. Specifically my += operator seems to have something wrong with it but I can't understand what.

I can't add in or change any of the prototypes and I should use C++ constructs over C type where possible.

Thanks!

#include <iostream>
#include <string>
#include "tstr.h"
using namespace std;

//Default constructor to initialize the string to null
TStr::TStr() {
    strPtr = 0;
    strSize = 0;
}  
//constructor; conversion from the char string
TStr::TStr(const char *str) {
    int i=0;
    while (str[i] != '\0') {
        ++i;
    }
    strSize = i;
    strPtr = new char [i+1];
    for (i=0; i < strSize; ++i) {
        strPtr[i] = str[i];
    }
} 
//Copy constructor
TStr::TStr(const TStr& str) {
    strPtr = new char[str.strSize];
    strcpy(strPtr, str.strPtr);
}
//Destructor
TStr::~TStr() {
    delete[] strPtr;
}

//subscript operators-checks for range
char& TStr::operator [] (int i) {
    assert (i >= 0 && i < strSize);
    return strPtr[i];
}
const char& TStr::operator [] (int i) const {
    assert (i >= 0 && i < strSize);
    return strPtr[i];
}

//overload the concatenation oprerator
TStr TStr::operator += (const TStr& str) {
    char *buffer = new char[strSize + str.strSize + 1];
    strcpy(buffer, strPtr);
    strcat(buffer, str.strPtr);
    delete [] strPtr;
    strPtr = buffer;
    return *this;
}

//overload the assignment operator
const TStr& TStr::operator = (const TStr& str) {
    if (this != &str) {
        delete[] strPtr;
        strPtr = new char[strSize = str.strSize];
        assert(strPtr);
        for (int i=0; i<strSize; ++i) {
            strPtr[i] = str.strPtr[i];
        }
    }
    return *this;
}

//overload two relational operators as member functions
bool TStr::operator == (const TStr& str) const {
    int value = strcmp(strPtr, str.strPtr);
    if (value == 0) {
        return true;
    } else {
        return false;
    }
    /*int counter=0;
    for (int i=0; i < strSize; ++i) {
        if (strPtr[i] == str.strPtr[i]) {
            ++counter;
        }
    }
    if (counter == strSize) {
        return true;
    } else {
        return false;
    }
    return (strPtr == str.strPtr && strSize == str.strSize);*/
}
bool TStr::operator < (const TStr& str) const {
    return (strPtr < str.strPtr && strSize < str.strSize);
}
//the length of the string
int TStr::size() {
    return strSize;
}

//Overload the stream insertion and extraction operators.
ostream& operator << (ostream& out, const TStr& str) {
int size = str.strSize;
for (int i=0; i < size; ++i) {
    out << str[i];
}
return out;
}
istream& operator >> (istream& in, TStr& str) {
return in;
}

//overload two other relational operators as global functions
bool operator != (const TStr& S1, const TStr& S2) {
    return !(S1 == S2);
}
bool operator <= (const TStr& S1, const TStr& S2) {
    return (S1 < S2 || S1 == S2);
}
bool operator > (const TStr& S1, const TStr& S2) {
    return !(S1 < S2);
}
bool operator >= (const TStr& S1, const TStr& S2) {
    return !(S1 < S2 || S1 == S2);
}

//overload the concatenation operator as a global function
TStr operator + (const TStr& str1, const TStr& str2) {
    //return (str1 += str2);
    //return (str1 + str2);
}

This is what I'm testing with:

int main() {
authors();
TStr str1 = "VENI";         //initialize str1 using 
                                //the assignment   operator 
const TStr str2("VEDI");    //initialize str2 using the
                                    //conversion constructor
TStr str3; //initialize str3 to null
TStr str4; //initialize str4 to null

cout << "\nTest 1: str1: " << str1 << " str2 " << str2 
     << " str3 " << str3 << " ###.\n" ;          //Test 1

if (str1 <= str2)                               //Test 2  
    cout << "\nTest 2: " << str1 << " is less "
         << "than " << str2 << endl;            
else                                            
    cout << "\nTest 2: " << str2 << " is less "
         << "than " << str1 << endl;            

str1=" Pride is what we have.";

str3 = str1;                                    //Test 3
cout << "\nTest 3: The new value of str3 = "
     << str3 << endl;                          


str3 += " Vanity is what others have. ";              
cout<<"\nTest 4: The str3: '" << str3<<"'  \nhas ";        //test 4
cout<< countVowels(str3)<<" vowels "<< endl;

/*TStr str5 = str1 + str2 + str3;
cout<<"\nTest 5: The str5: '" << str5<<"' \nhas ";        //test 5
cout<<countVowels(str5)<<" vowels \n";

cout<<"\nTest 6: The str3 again: " << str3 <<endl;    //test 6

cout<<"\n\n Bye, Bye!";*/


return 0;
}

EDIT: I should have added that I need to get the tests to work as part of the homework.

EDIT 2: Here are the functions I changed so far.

//constructor; conversion from the char string
TStr::TStr(const char *str) {
    int i=0;
    while (str[i] != '\0') {
        ++i;
    }
    strSize = i;
    strPtr = new char [i+1];
    for (i=0; i < strSize; ++i) {
        strPtr[i] = str[i];
        strPtr[i + 1] = '\0'; //<- this
    }
} 
//Copy constructor
TStr::TStr(const TStr& str) {
    strPtr = new char[str.strSize + 1]; //<-this
    strcpy(strPtr, str.strPtr);
}
//overload the concatenation oprerator
TStr TStr::operator += (const TStr& str) {
    char *buffer = new char[strSize + str.strSize + 1]; //<- this
    strcpy(buffer, strPtr);
    strcat(buffer, str.strPtr);
    delete [] strPtr;
    strPtr = buffer;
    return *this;
}

//overload the assignment operator
const TStr& TStr::operator = (const TStr& str) {
    if (this != &str) {
        delete[] strPtr;
        strPtr = new char[strSize = str.strSize + 1]; //<- this
        assert(strPtr);
        for (int i=0; i<strSize; ++i) {
            strPtr[i] = str.strPtr[i];
        }
    }
    return *this;
}

EDIT 3: Unfortunately changing it to below did nothing really. It might be the = operator that's causing the problem as it now fails just after test 3.

//constructor; conversion from the char string
TStr::TStr(const char *str) {
    int i=0;
    while (str[i] != '\0') {
        ++i;
    }
    strSize = i;
    strPtr = new char [i+1];
    strcpy(strPtr, str);
} 
RedFred
  • 67
  • 1
  • 8
  • Have you debugged your program? Tip: you should pay attention to how you pair your "new" and "delete" ... and debug it to see what's going wrong. – celavek Aug 28 '11 at 22:18
  • What is the error you are receiving? – Brian Roach Aug 28 '11 at 22:22
  • None of your code is exception-safe. I strongly recommend that you read about [the copy-and-swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) – Ben Voigt Aug 28 '11 at 22:28
  • Using GCC debug "0x4016b6
    : lea 0xfffffff4(%ebp),eax" I don't really know how to use that program and I can't get it out of assembly language.
    – RedFred Aug 28 '11 at 22:28

5 Answers5

1

You have several problems involving the null-terminator with several of your functions(some of these you have fixed in your edits, but I'm just going to cover them all anyway, so that this is a thorough answer).

  1. In the constructor converting from const char *, you are not copying over the null-terminator to strPtr. (fixed)
  2. In your copy constructor, you are not allocating enough room for the null-terminator. (fixed)
  3. In your assignment operator, you are not allocating enough room for the null-terminator. (fixed ... not exactly, see #5)
  4. In your assignment operator, you are not copying over the null-terminator.
  5. You added a new problem in your assignment operator when you fixed the allocation. You are now setting strSize to the wrong size.
Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274
  • The copy constructor is only used for generating the return value, which is discarded. – Ben Voigt Aug 28 '11 at 22:26
  • While this is a bug in the copy constructor ... how is `+=` relying on it? I suspect the issue is that his test is wrong; it's not appendng another `TStr` but rather a string literal. – Brian Roach Aug 28 '11 at 22:29
  • Hmm I added the +1 but it still doesn't add the new string to the old string. – RedFred Aug 28 '11 at 22:30
  • @Ben: I'm looking at the code he commented out, which I assume is where he saw a problem. It uses `operator+`, which relies on the return value of `operator+=`. – Benjamin Lindley Aug 28 '11 at 22:35
  • @RedFred: Did you also fix your assignment operator and your constructor taking `const char *`? – Benjamin Lindley Aug 28 '11 at 22:55
  • As I said in the OP, I can't change the prototypes. So as they are written is how I have to use them, same deal with the tests. I have made all the changes people have but nothing seems to have fixed the problem. My code fails earlier now. Help! – RedFred Aug 28 '11 at 23:04
  • @RedFred: I said nothing about changing the prototypes. Can I see the changes you've made so far? – Benjamin Lindley Aug 28 '11 at 23:07
  • @RedFred: The fix on your constructor from `const char*` is no good. Try this: http://ideone.com/5z7TX – Benjamin Lindley Aug 28 '11 at 23:18
  • @RedFred: I didn't see this at first. Your assignment operator has the same problem with not null-terminating the string. – Benjamin Lindley Aug 28 '11 at 23:38
  • @RedFred: Please see my answer for updates. I think I covered all your problems now. – Benjamin Lindley Aug 29 '11 at 01:04
1

There are several places in which you need +1 to reserve the ending \0 for strings. For example, in the copy constructor

strPtr = new char[str.strSize];

(that should be str.strSize+1) and in operator=(). These may be the sources of your problems.

BUT you should carefully review all these kind of errors, and test each function separately (maybe with a good test suite, you know, written before the class itself) :)

Diego Sevilla
  • 28,636
  • 4
  • 59
  • 87
1

Your char* converting constructor doesn't NUL-terminate the content (it reserves space for a NUL, but doesn't store one), which causes strcpy and strcat to fail later.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • Would adding the line 'strPtr[i + 1] = '\0';' as the second line in the for loop work? – RedFred Aug 28 '11 at 22:44
  • @RedFred: That would work, but it would write many extra characters that would just get overwritten. All you need is `strPtr[strSize] = 0;` AFTER the `for` loop. – Ben Voigt Aug 28 '11 at 23:24
1

Since no one has pointed it out yet, your strSize remains undefined in your copy constructor; and in your assignment operator.

You only need to null terminate your strings if they will be interfacing with C style strings (ie char*, strcopy etc).

hiddensunset4
  • 5,825
  • 3
  • 39
  • 61
0

operator +=() should return a reference -- TStr&. You are returning a copy, which is probably not what you want.

tenfour
  • 36,141
  • 15
  • 83
  • 142
  • Okay so I changed it to 'return strPtr;' and it sort of works but I must have a memory leak or something as the string it adds is a random value. – RedFred Aug 28 '11 at 23:44
  • I can't. I'm not allowed to change any of the prototypes. So would that work if it has to stay as non reference? – RedFred Aug 28 '11 at 23:59