0

I have a problem about char*, string add together such like this:

enter code here
s2 = s3 + "," + s1; 

and I have three operator below

friend Mystring operator+( const Mystring &lhs, const Mystring &rhs);  -- 1
friend Mystring operator+( const Mystring &mystr, const char *ch ); -- 2
friend Mystring operator+( const char *ch, const Mystring &mystr ); -- 3

but I use 1 and 3 it will crash, but I use 1 and 3 can do good.

My problem is the order isn't that s3 + "," first so use operator w first and the result use operator 3, but the fact isn't as my thought.

Can anyone explain why this happens?

Mystring operator+( const Mystring &mystr,const char *ch )
{
  Mystring tmp;
  tmp.str_ = new char[ strlen(mystr.str_)+2 ];
  strcpy( tmp.str_, mystr.str_ );
  strcat( tmp.str_, ch );
  return tmp;
}


Mystring operator+( const char *ch, const Mystring &mystr )
{
  Mystring tmp;
  tmp.str_ = new char[ strlen(mystr.str_)+strlen(mystr.str_)+1 ];
  strcpy( tmp.str_, mystr.str_ );
  strcat( tmp.str_, mystr.str_ );
  return tmp;
}

Mystring operator+( const Mystring &lhs, const Mystring &rhs )
{
  Mystring tmp;
  tmp.str_ = new char[ strlen(lhs.str_)+strlen(rhs.str_)+1 ];
  strcpy( tmp.str_, lhs.str_ );
  strcat( tmp.str_, rhs.str_ );
  return tmp;
}
Unapiedra
  • 15,037
  • 12
  • 64
  • 93
Coding Fox
  • 11
  • 2
  • 2
    Can you post the definitions of each operator as well as the exact error your getting? – MGZero Dec 05 '11 at 14:21
  • 2
    Your question isn't clear. What do you mean by "I use 1 and 3 it will crash, but I use 1 and 3 can do good"? – Oliver Charlesworth Dec 05 '11 at 14:21
  • What's the definition of `Mystring`? – In silico Dec 05 '11 at 14:22
  • 1
    you `new` a lot of char but you never `delete`. Writing your own string class is not for beginners. Use std::string and choose another utility class to implement for yourself - strings are gnarly. – Kate Gregory Dec 05 '11 at 14:37
  • 2
    You can easily reduce that to one overload - `Mystring operator+(Mystring const& lhs, Mystring const& rhs)` - if you have a non-explicit constructor from `char const*`. – Xeo Dec 05 '11 at 14:37
  • 4
    The canonical way to implement `operator+` is atop of `operator+=`. See http://stackoverflow.com/questions/4421706/operator-overloading – sbi Dec 05 '11 at 14:48

3 Answers3

2

Try testing simpler things first:

s2 = s3 + ",";
s2 = "," + s3;
s3 = s1 + s2;

before moving on to chaining:

s2 = s3 + ","  + s1;

that way you can tell what the issue is more clearly.

Kate Gregory
  • 18,808
  • 8
  • 56
  • 85
1

In

Mystring operator+( const Mystring &mystr,const char *ch )
{
  Mystring tmp;
  tmp.str_ = new char[ strlen(mystr.str_)+2 ];

you should write:

  tmp.str_ = new char[ strlen(mystr.str_) + strlen(ch) + 1 ];

And here:

Mystring operator+( const char *ch, const Mystring &mystr )
{
  Mystring tmp;
  tmp.str_ = new char[ strlen(mystr.str_)+strlen(mystr.str_)+1 ];

you should write:

  tmp.str_ = new char [ strlen(ch) + strlen(mystr.str_) + 1 ];
Andrea Bergia
  • 5,502
  • 1
  • 23
  • 38
  • 1
    Teaching C++ I learned (the hard way) that, if I want to teach my students how to _overload operators_ by them implementing their own string class, I'd better advise them to use `std::vector` for storing their data, and _concentrate on operator overloading_. Managing data manually should be the next step, with the students concentrating on manual resource management. Doing both at the same time was overloading most of them. – sbi Dec 05 '11 at 14:45
  • 1
    If I was to teach operator overload, I'd start with complex, or rational numbers. Far simpler to get right than strings, IMVHO. – Andrea Bergia Dec 05 '11 at 14:49
0

Have you read this? What is The Rule of Three?

Based on the way you manually manage memory in these functions, I think it's safe to assume that you haven't. Anyway, once you do properly implement the big three, here is what you should do. First you should have two integral members. One to store the size of the string, and one to store the capacity of the dynamic array. Then you want to have a private function, let's call it increase_capacity, that increases the capacity if necessary. All your memory allocation will take place there, in one spot. It will simplify things greatly. It might look something like this:

void increase_capacity(int cap)
{
    if (cap <= capacity_) return;
    char * temp = new char[cap];
    capacity_ = cap;
    memcpy(temp, str_, size_); // Or size_ + 1 if your string is null terminated.
                               // It doesn't have to be.
    delete [] str_;
    str_ = temp;
}

Now, you should also have a resize function, that adjusts the size, if necessary.

void resize(int size)
{
    if (size > capacity_)
    {
        int cap = capacity_ * 2;
        cap = cap > size ? cap : size;
        increase_capacity(cap);
    }

    size_ = size;
    // Fill in new elements with some default value, or don't.
}

Better than all that above, you should just use vector<char>, but maybe you're trying to understand manual memory management, that's fine.

Now, you should implement operator+=, as a member:

Mystring & operator+=(const Mystring & rhs)
{
    int old_size = size_;
    resize(size_ + rhs.size_);
    memcpy(str_ + old_size, rhs.str_, rhs.size_);
    return *this;
}

If you want, you can also implement one that takes const char *, that will save an allocation that would happen if you relied on the implicit conversion.

Finally, you can implement your operator+:

Mystring operator+(Mystring lhs, const Mystring & rhs)
{
    return lhs += rhs;
}

If you have an implicit conversion from const char *, that should cover all of them. But if you wrote the extra operator+= that takes a const char * in order to do fewer allocations, you should probably also write the one that takes const char * on it's right side. It looks the same as the one above, just the type of the second parameter changes. You shouldn't need to write one for the reverse operation, since the lhs will need to be allocated as a Mystring anyway.

Community
  • 1
  • 1
Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274