Проверка класса динамического массива строк

Рейтинг: 2Ответов: 1Опубликовано: 25.01.2015

Проверьте пожалуйста код программы, написанный мной по следующему условию:

Составить описание класса одномерных массивов строк, каждая строка задается длиной и указателем на выделенную для нее память. Предусмотреть возможность обращения к отдельным строкам массива по индексам, контроль выхода за пределы массивов, выполнения операций поэлементного сцепления двух массивов с образованием нового массива, слияния двух массивов с исключением повторяющихся элементов, вывод на экран элемента массива и всего массива. Написать программу, демонстрирующую работу с этим классом. Программа должна содержать меню, позволяющее осуществить проверку всех методов класса.

Код:

#include "stdafx.h"
#include <iostream>

using namespace std;

class StringArray{

    char** string;

    int lenght=0;

    public: StringArray(){

    }

    void addString(char* str){

        this->lenght++;
        char** temp = new char*[this->lenght];

        for (int i = 0; i < this->lenght - 1; i++)
            temp[i] = string[i];

        delete string;
        string = temp;
        string[this->lenght - 1] = str;

    }

    StringArray mergeWith(StringArray a){

        StringArray res = StringArray();

        for (int i = 0; i < this->getLenght(); i++)
            res.addString(this->getString(i));

        for (int i = 0; i < a.getLenght(); i++)
            res.addString(a[i]);

        return res;
    }

    StringArray mergeWith(StringArray a, bool is_unique){

        if (!is_unique) return this->mergeWith(a);

        StringArray res = StringArray();

        for (int i = 0; i < this->getLenght(); i++)
             if (!res.searchString(this->getString(i))) res.addString(this->getString(i));

        for (int i = 0; i < a.getLenght(); i++)
            if (!res.searchString(a.getString(i)))  res.addString(a[i]);

        return res;
    }

    int getLenght(){
        return this->lenght;
    }

    char* getString(int i){

        if (i < this->lenght)
        return  string[i];

        return "Неверный индекс";
    }

    bool searchString(char* str){

        for (int i = 0; i < this->lenght; i++)
            if (this->getString(i) == str)
                return true;

        return false;
    }

    char* operator[](int i){
        return this->getString(i);
    }

    StringArray operator +(StringArray&a){

        return this->mergeWith(a);
    }

friend ostream &operator <<(ostream &out,StringArray &a){
    for (int i = 0; i < a.getLenght(); i++)
        cout << a[i] << endl;

    return out;
    }

};

int main(int argc, _TCHAR* argv[])
{

    StringArray arr1 = StringArray();
    arr1.addString("первая");
    arr1.addString("вторая");
    arr1.addString("третья");

    StringArray arr2 = StringArray();
    arr1.addString("третья");
    arr1.addString("четвертая");

    cout << "Первый элемент: " << arr1[0] << endl << "Второй элемент: " << arr1.getString(1) << endl;

    StringArray merged = arr1.mergeWith(arr2,true);

    cout << merged << endl;

    cout << arr1 + arr2 << endl;

    int n;
    cin >> n;
    return 0;
}

Какие будут замечания и предложения по коду? :)

Ответы

▲ 2
  1. Функция addString реализован ужасно с точки зрения производительности. При добавлении новой строки вы каждый раз стираете удаляете имеющиеся данные и циклом по всем строкам заполняете массив теми же значениями, что в нем были раньше плюс добавляете новую строку. Вопрос - зачем каждый раз делать бесполезную работу (удалять содержимое и писать его заново), при этом в каждом случае проходя по всему массиву. Правильнее было бы создать масси и при добавлении строки писать ее в свободную ячейку массива, а когда массив будет заполнен, выделить новый массив большего размера с копированием в него существующего и последующим удалением существующего.
  2. Два цикла в функции mergeWith выглядят странно. Достаточно было бы и одного (или может даже вообще какого-нибудь хитрого колдунства с указателями)
  3. У вас есть две перегрузки функции mergeWith. В обоих присутствует энное количество дублирующейся логики, хотя они отличаются лишь одним параметром. Следовательно, при внесении изменений вам понадобится менять обе. Правильно было бы сделать, чтобы первая перегрузка сводилась к вызову второй с is_unique, равным false
  4. В случае, если аргумент функции getString больше чем размер массива, зачем-то возвращает строку "Неверный индекс". Это ооочень неправильно. По хорошему она должна выкидывать исключение или хотя бы возвращать NULL.
  5. В этой же функции вы не предусмотрели, что будет, если аргумент будет отрицательным. Также скорее всего случится что-нибудь нехорошее, если передать туда аргумент, равный размеру массива
  6. Оператор operator << крашится, если в массиве присутствует NULL
  7. Я не знаток С++, но по-моему, параметры там лучше передавать с помощью константных ссылок
  8. В вашем задании говорится: " каждая строка задается длиной и указателем". Возможно я не заметил, но по-моему, вы нигде не контролируете длины строк, а только длину всего массива

Из мелочей:

  1. Ненужный конструктор
  2. Название функции searchString не интуитивно понятно. Если функция называется searchString, то логично, что возвращать она должна строку, а не bool. Правильнее было бы назвать ее contains или как-то наподобие
  3. Правильно писать length, а не lenght