In addition to what nv3 said, whenever you're doing any low level resource management task like this (you're essentially managing a variable sized array of characters) make sure you take exception safety into account.
Looking at nv's modifications:
void string1::join (const string1 &a, const string2 &b)
{
length = a.length + b.length;
delete [] name;
name = new char[c.length + 1];
strcpy (name, a.name);
strcat (name, b.name);
}
ask yourself what happens to the object you've called the function on if the
new
throws? The answer is you'll end up with the object having a screwy state - the length suggests that there's a valid string but it's got a pointer to some deleted memory. Ugh. Makes recovery impossible and you have to bin the object.
How to get around this is hinted in SA's answer: Write a constructor. A handy guideline is do all
new
s in a constructor and all
delete
s in the destructor. You won't be able to manage it all the time but it's a handy rule to keep in mind when you code.
Why? If constructors fail then the object never exists and
the rest of the program state can stay the same. That makes it far easier to recover or report errors - you haven't scrambled anything apart from the object you were constructing.
So how could you implement
join
using a constructor? Well first off write a two argument constructor:
string1( const string1 &a, const string1 &b )
{
length_ = a.length + b.length;
data_ = new char[ length + 1 ];
std::copy( a.name, a.name + a.length, name );
std::copy( b.name, b.name + b.length_, name + a.length );
name[ length ] = 0;
}
Then define a swap function:
void swap( string1 &swap_with )
{
std::swap( length, swap_with.length );
std::swap( name, swap_with.name );
}
and your
join
becomes:
void join( const string1 &a, const string1 &b )
{
string1 temp( a, b );
swap( temp );
}
If the constructor goes horribly wrong your original object is still intact and in a usable state, hurrah!
This copy and swap is useful in other places as well - e.g. when you're writing assignment operators - so getting it fixed in your mind isn't a waste of time for any C++ programmer.
PS: In reply to comment...
Your ambition for
join
seems to be
+=
in most string implementations. You can do it with the two argument constructor and swap:
void join( const string1 &to_append )
{
string1 temp( *this, to_append );
swap( temp );
}
Scarily similar isn't it?
Basically sort out the constructors, a destructor and non-throwing swap and most copying operations become a doddle - and exception safe as well.