There is no point in having this constructor.
String::String()
You can simulate its affects by putting a default parameter in this constructor.
class String
{
public:
explicit String(int size = 0); // Have a default parameter and
// Covers both your first two constructors.
For the next constructor:
String::String( const int size )
^^^^^ I see little point in this.
Use initializer list when you can.
This:
{
m_size = size;
m_characters = new char[size];
}
Should be:
: m_size(size)
, m_characters(new char[size])
{}
In this case it makes no difference (I will admit). But not doing it will make you classes more brittle in for future modifications. So it is always a good habit to get into.
I would argue that this constructor should test for nullptr
String::String( const char* str )
Now the standard does not. But there have been many/many arguments over this. I think you will reduce errors be detecting this and shutting them down.
Don't try and get string size yourself.
m_size = 0;
int i = 0;
while ( str[i] )
{
m_size++;
i++;
}
There is a standard library function for this strlen(). On some systems it is even highly optimized to be quicker than stuff you can write using standard C. So use the system version it will potentially be quicker than stuff you write.
There is a standard library function for doing a copy:
for (int i = 0; i < m_size; i++)
m_characters[i] = str[i];
Prefer:
std::copy(m_characters, m_characters + m_size, str);
It will not be slower than your code but could be quicker. It also expresses intent much more clearly.
Same comments as above:
String::String( const String& string )
// Prefer initializer list.
{
m_size = string.m_size;
m_characters = new char[m_size];
// Use standard algorithms.
for ( int i = 0; i < m_size; i++ )
m_characters[i] = string.m_characters[i];
}
Destructor is very standard and will work.
String::~String()
{
delete [] m_characters;
}
The standard pattern for the assignment operator is Copy and Swap Idiom (please look it up).
String& String::operator=( const String& string )
{
// Suspect check for self assignment.
// Does it really make things quicker?
if ( this != &string )
{
m_size = string.m_size;
// This leaks the old value.
m_characters = new char[m_size];
for (int i = 0; i < m_size; i++ )
m_characters[i] = string.m_characters[i];
}
return *this;
}
The check for self assignment. Yes it looks like it speeds the code up when you do self assignment. It does. But conversely it slows the code down when you don't have self assignment (not very much but a tiny bit). The problem is that actual self assignment is very exceptionally rare.
So the question becomes: Is the size of number of non self assignment that happen per self assignment multiplied by a tiny cost smaller than 1 * cost of making a copy an average string. People have tried this and found the most efficient version is not to test for self assignment but always make a copy.
String& String::operator=(String string) // Pass by value to get a copy.
{
string.swap(*this);
return *this;
}
I would also note that you missed the move operators. Move semantics can improve the performance a bit as they avoid copying in a couple of situations).
