Why does not this main test program work

Discussion in 'C++' started by Tony Johansson, Dec 30, 2005.

  1. 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::eek:perator=(const String& s)
    {
    if (buffer)
    delete [] buffer;
    buffer = new char[strlen(s.buffer)+1];
    strcpy(buffer, s.buffer);
    return *this;
    }

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

    String String::eek: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::eek: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
    Tony Johansson, Dec 30, 2005
    #1
    1. Advertising

  2. Tony Johansson

    mlimber Guest

    Tony Johansson wrote:
    > 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
    mlimber, Dec 30, 2005
    #2
    1. Advertising

  3. Tony Johansson wrote:
    > 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::eek: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::eek:perator==(const String& s) const
    > {
    > if (strcmp(buffer, s.buffer) == 0)
    > return 1;
    > else
    > return 0;
    > }


    this should be

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

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


    Idem.

    > String String::eek: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::eek: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
    Jonathan Mcdougall, Dec 30, 2005
    #3
  4. Tony Johansson

    Heinz Ozwirk Guest

    "Tony Johansson" <> schrieb im Newsbeitrag
    news:sOhtf.153151$...
    > 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::eek: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::eek: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::eek: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::eek: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::eek: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
    Heinz Ozwirk, Dec 30, 2005
    #4
  5. Tony Johansson

    David Harmon Guest

    On Fri, 30 Dec 2005 21:39:36 GMT in comp.lang.c++, "Tony Johansson"
    <> wrote,
    >String String::eek: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.
    David Harmon, Dec 30, 2005
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Horace Nunley

    why why why does function not work

    Horace Nunley, Sep 27, 2006, in forum: ASP .Net
    Replies:
    1
    Views:
    452
    =?Utf-8?B?UGV0ZXIgQnJvbWJlcmcgW0MjIE1WUF0=?=
    Sep 27, 2006
  2. Mr. SweatyFinger

    why why why why why

    Mr. SweatyFinger, Nov 28, 2006, in forum: ASP .Net
    Replies:
    4
    Views:
    863
    Mark Rae
    Dec 21, 2006
  3. Mr. SweatyFinger
    Replies:
    2
    Views:
    1,764
    Smokey Grindel
    Dec 2, 2006
  4. Phi!
    Replies:
    1
    Views:
    172
  5. Peter Recore
    Replies:
    1
    Views:
    102
    Christopher Dicely
    Feb 16, 2008
Loading...

Share This Page