MyString::MyString(char * n) //copy constructor
{
size=strlen(n);
capacity=strlen(n)+1;
data=new char[capacity];
strcpy(data,n);
}
Нет смысла звонить strlen дважды.Оптимизаторы недостаточно умны, чтобы устранить избыточность.Кроме того, поскольку вы не изменяете данные в переданной строке, это должно быть const.В противном случае вы не могли бы передать это строковые константы.Исправляя, мы получаем:
MyString::MyString(const char * n) //copy constructor
{
size = strlen(n);
capacity = size + 1;
data = new char[capacity];
strcpy(data, n);
}
MyString::MyString(const MyString &right) //
{
size=strlen(right.data);
capacity=strlen(right.data)+1;
data=new char [capacity];
strcpy(data,right.data);
}
right имеет size член, который устраняет необходимость в strlen.Это плюс приведенные выше исправления дают:
MyString::MyString(const MyString &right) //
{
size = right.size;
capacity = size + 1;
data = new char[capacity];
strcpy(data, right.data);
}
MyString MyString::operator = (const MyString& s)
{
if(this!=&s)
{
MyString temp=data;
delete [] data;
size=strlen(s.data);
capacity=size+1;
data= new char [capacity];
strcpy(data,s.data);
}
}
operator = должен возвращать ссылку (т. Е. *this; кстати, вы забыли что-либо вернуть), а не другую копию.Также обратите внимание, что temp не используется, и у вас есть избыточный strlen вызов:
MyString& MyString::operator = (const MyString& s)
{
if(this!=&s)
{
delete[] data;
size = s.size;
capacity = size + 1;
data = new char[capacity];
strcpy(data, s.data);
}
return *this;
}
MyString& MyString::append(const MyString& s)
{
if(this!=&s)
{
strcat(data,s.data);
}
}
Вы забыли проверить свои size и capacity.Так что strcat будет запускаться прямо в конец вашего char массива.Кроме того, объединение к себе должно быть сделано для работы.И вам не нужно strcat:
MyString& MyString::append(const MyString& s)
{
size_t rSize = s.size;
if(capacity < size + rSize + 1)
{
capacity = size + rSize + 1;
char* newData = new char[capacity];
memcpy(newData, data, size);
delete[] data;
data = newData;
}
memcpy(data + size, s.data, rSize);
size += rSize;
data[size] = '\0';
return *this;
}
erase, просто нужно, чтобы строка ушла.Нет необходимости перепутывать память с:
MyString& MyString::erase()
{
size = 0;
data[0] = '\0';
}
MyString MyString::operator + (const MyString& s)const
{
return strcat(data,s.data);
}
Те же проблемы, что и с append, плюс этот должен возвращать новый объект и оставлять его входные данные в покое.Кроме того, используйте методы, которые были написаны до сих пор:
MyString MyString::operator + (const MyString& s) const
{
return MyString(*this).append(s);
}
void MyString::operator += (const MyString& s)
{
append(s.data);
}
Это коварный способ!Видите ли, поскольку append принимает const MyString&, а вы присваиваете ему char*, компилятор создает временный MyString, вызывает для него конструктор MyString(const char*), передает временное значение в append, затем сбрасываетЭто.Таким образом, вы получите правильный результат, но вы создали дополнительный объект.Правильный путь:
void MyString::operator += (const MyString& s)
{
append(s);
}
char& MyString::operator [ ] (int n)
{
return data[n];
}
Нет проверки границ?Храбрый.Если вы хотите, это не сложно добавить.Предполагая, что вы не хотите генерировать исключение:
char& MyString::operator [] (int n)
{
if(n < 0)
n = 0;
if(n >= size)
n = size - 1;
return data[n];
}
FYI, обычный тип индекса - size_t, который определен в <stddef.h> и является целочисленным типом без знака, возвращаемым sizeof(), так что он гарантированно будет работать как индекс массива с обычным массивом (здесь вы можете избавиться от отрицательной проверки индекса).
Кроме того, вы можете захотеть версию, которая может работать на const MyString,Он просто возвращает символ вместо ссылки:
char MyString::operator [] (int n) const
{
if(n < 0)
n = 0;
if(n >= size)
n = size - 1;
return data[n];
}
void MyString::getline(istream& in)
{
char c;
erase();
ifstream input;
while(in.get(c)&&c!='\n')
{
data[size]=c;
size++;
if(size+1<=capacity)
{
capacity*=2;
char*p=new char[capacity];
strcpy(p,data);
delete [] data;
data=p;
}
data[size]=c;
size++;
data[size]='\0';
}
}
Сначала ifstream ничего не делает;избавиться от этого.Вы должны работать только с istream.Индивид указал на проблемы с дополнительными символами и плохой проверкой.strcpy потерпит неудачу, потому что вы не поместили \0 обратно в конец строки, поэтому вам нужно использовать memcpy вместо:
void MyString::getline(istream& in)
{
char c;
erase();
while(in.get(c) && c != '\n')
{
data[size++] = c;
if(size >= capacity)
{
capacity *= 2;
char *newData = new char[capacity];
memcpy(newData, data, size);
delete[] data;
data = newData;
}
}
data[size] = '\0';
}
int MyString::length( ) const
{
return strlen(data);
}
Thisпросто тратить время, потому что size уже имеет длину строки.Он действительно должен быть определен в определении класса, поэтому он может быть встроен:
int MyString::length() const
{
return size;
}
void MyString::grow()
{
capacity=strlen(data)+1;
MyString temp;
temp=data;
delete [] data;
capacity*=2;
data= new char[capacity];
}
Первая строка просто неверна;capacity уже настроен правильно.Вам не нужно temp.И вы забыли скопировать строковые данные поверх:
void MyString::grow()
{
capacity *= 2;
char* newData = new char[capacity];
memcpy(newData, data, size + 1);
delete[] data;
data = newData;
}
ostream& operator<<(ostream& out, MyString& s)
{
out<<s.data;
return out;
}
Вы не меняете s, поэтому должно быть const:
ostream& operator<<(ostream& out, const MyString& s)
{
return out << s.data;
}