Проектирование многомерного массива в C ++ - PullRequest
2 голосов
/ 07 июля 2011

Я хочу создать класс C ++ для многомерных массивов.Под многомерным я имею в виду 1d, 2d, 3d и т. Д. Класс поддерживает поэлементное сложение и операторы скалярного умножения.Предположим, что A и B являются экземплярами класса (одинакового размера и размера).Я хотел бы использовать объекты в выражениях, подобных этому:

C = A * 2 + B

Меня интересует, как управлять памятью.В приведенном выше выражении A * 2 создаст временный объект класса, который позже будет добавлен в B. В любом случае, временный объект становится мусором после того, как добавление выполнено.Я написал следующий класс, и он хорошо работает, но я почти уверен, что утечка памяти.Теперь мои вопросы:

  1. Как я могу исправить проблему с памятью?Каков наилучший способ создания класса?
  2. Можно ли выделить требуемую память в стеке вместо кучи?

    class XArray
    {
        int num_dim;
        int *dims;
        int *index_helper;
        int table_size;
        double *table;
    
    
    public:
        XArray(const int n, const int *d):num_dim(n), dims(d)
        {
            int size = 1;
            for (int i = 0; i < n; i++) {
                size *= d[i];
            }
            table_size = size;
            table = new double[size];
            index_helper = new int[n];
        };
    
        ~XArray()
        {
            delete[] table;
            delete[] index_helper;
        };
    
        int dim(int d)
        {
            return dims[d];
        }
    
        double& operator()(int i)
        {
            index_helper[0] = i;
            return get_helper(1, index_helper);
        }
    
        double& operator()(int i, int j)
        {
            index_helper[0] = i;
            index_helper[1] = j;
            return get_helper(2, index_helper);
        }
    
        double& operator()(int i, int j, int k)
        {
            index_helper[0] = i;
            index_helper[1] = j;
            index_helper[2] = k;
            return get_helper(3, index_helper);
        }
    
        XArray operator*(double m)
        {
            XArray *xa = new XArray(num_dim, dims);
            for (int i = 0; i < table_size; i++) {
                xa->table[i] = this->table[i] * m;
            }
    
            return *xa;
        }
    
        XArray operator+(const XArray &that)
        {
            if (num_dim != that.num_dim) {
                char *msg = new char[100];
                sprintf(msg, "XArray::dimensions do not match in + operation, expected %d, found %d", num_dim, that.num_dim);
                throw msg;
            }
    
            for (int i = 0; i < num_dim; i++) {
                if (this->dims[i] != that.dims[i]) {
                    char *msg = new char[100];
                    sprintf(msg, "XArray::dimension %d not mached, %d != %d", i, dims[i], that.dims[i]);
                    throw msg;
                }
            }       
    
            XArray *xa = new XArray(num_dim, dims);
            for (int i = 0; i < table_size; i++) {
                xa->table[i] = this->table[i] + that.table[i];
            }
    
            return *xa;
        }
    
    private:
        double& get_helper(int n, int *indices)
        {
            if (n != num_dim) {
                char *msg = new char[100];
                sprintf(msg, "XArray::dimensions do not match, expected %d, found %d", num_dim, n);
                throw msg;
            }
    
            int multiplier = 1;
            int index = 0;
    
            for (int i = 0; i < n; i++) {
                if (indices[i] < 0 || indices[i] >= dims[i]) {
                    char *msg = new char[100];
                    sprintf(msg, "XArray::index %d out of range, %d not in (0, %d)", i, indices[i], dims[i]);
                    throw msg;
                }
    
                index += indices[i] * multiplier;
                multiplier *= dims[i];
            }
    
        return table[index];
    }
    

    };

Ответы [ 3 ]

4 голосов
/ 07 июля 2011

Мне не хватает конструкторов копирования и операторов присваивания. Они понадобятся вам для создания копий внутренних буферов, а не просто для копирования указателей, иначе вы в конечном итоге освободите одну и ту же память дважды.

Если вы собираетесь поддерживать c ++ 0x, вам также может понадобиться реализовать семантику перемещения для оптимальной производительности.

Бросать динамически размещенные объекты, как вы делаете, очень плохая идея.

В вашем операторе * вы должны создать объект в куче:

XArray operator*(double m)
{
    XArray xa(num_dim, dims);
    for (int i = 0; i < table_size; i++) {
        xa->table[i] = this->table[i] * m;
    }

    return xa;
}

Однако, если вы не напишите свои (перемещать) конструкторы копирования, это будет зависать. Потому что оператор return сделает копию, которая ссылается на те же внутренние данные, что и xa. xa будет уничтожен, а его деструктор уничтожит его внутренние данные. Поэтому возвращаемый временный объект внутренне указывает на уничтоженные данные.

редактирование:

Ссылка на информацию о Перемещение семантики или rvalue ссылок

2 голосов
/ 07 июля 2011

Я бы предложил следующее, чтобы избежать утечек памяти:

  1. Сделать копировать конструктор и operator = () private и не реализовано, чтобы избежать перезаписи table и index_helper (и случайно утечки памяти). Если вы хотите их использовать, то убедитесь, что , вы делаете delete[] table и delete[] index_helper; перед переназначением, чтобы избежать утечки.
  2. Для сообщений об ошибках используйте std::string вместо char *msg = new char[100];. Это чище и ремонтопригодны.
  3. Не использовать XArray *xa = new XArray(num_dim, dims);; вместо просто объявите это как XArray xa;. Это будет в стеке и заведено автоматически (если у вас есть поддержка c ++ 0x, вы можете выбрать идеальную пересылку для удаления ненужных копий)

Теперь, что касается оставшегося class дизайна, ответ будет очень субъективным и потребует детального анализа кода. Итак, я оставлю это на ваше усмотрение.

1 голос
/ 07 июля 2011

Этот вопрос, вероятно, должен быть в Code Review , а не здесь.Но некоторые вещи в дизайне:

Обычно не рекомендуется управлять памятью вручную, предпочтительнее использовать существующие контейнеры вместо динамически размещаемых массивов.Тот факт, что конструктор получает право владения переданным указателем, не является распространенным и может привести к проблемам с пользовательским кодом.В настоящее время вы запрещаете некоторые виды использования, например:

int dims[3] = { 3, 4, 5 };
XArray array( 3, dims ); // or use sizeof(dims)/sizeof(dims[0]), or other trickery

Что проще для пользовательского кода, чем:

int ndims = 5;
int *dims = new int[ndims]
XArray array( ndims, dims );

Это также означает, что пользователь должен знать, что владение dims имеети что они не могут delete указатель.

Внутренне я бы использовал std::vector для поддержки динамической памяти, поскольку это даст вам правильную семантику бесплатно.Так как это вам нужно реализовать конструктор копирования и оператор присваивания (или отключить их), так как текущий код не будет течь, но может попытаться удвоить часть памяти.Запомните правило трех: если вы предоставляете любой из конструктор копирования , оператор присваивания или деструктор , вам, вероятно, следует предоставить все три.

Ваша operator* и operator+ утечка памяти (сам объект XArray, поскольку внутренняя память фактически обрабатывается из-за отсутствия конструктора копирования, но не думайте, что это не причиначтобы создать конструктор копирования, вы, напротив, должны создать его)

Бросок char* разрешен, но я бы порекомендовал вам этого не делать по разным причинам.Тип исключения должен содержать информацию о том, что произошло, иначе ваш код пользователя должен будет проанализировать содержимое исключения, чтобы определить, что пошло не так.Даже если вы решите использовать один тип, обратите внимание, что те же правила, которые используются при разрешении перегрузки, не применяются при обработке исключений, и, в частности, у вас могут возникнуть проблемы с отсутствующими преобразованиями.Это также еще один потенциал для утечки памяти в вашем проекте, так как при создании указателя на динамически выделяемую память вы делегируете ответственность за освобождение памяти вашим пользователям, если пользователь не освобождает память или пользователь на самом деле этого не делает.позаботьтесь об исключении и просто перехватите все (catch (...) {}), вы потеряете ошибки.

Возможно, будет лучше, если вместо throw ing вас assert ed, если это возможно в вашей программе (это дизайнерское решение: насколько плох неправильный размер, что может случиться, даже если оно исключительное, или что не должно происходить?).

Член index_helper не принадлежит классу, этоэто деталь реализации, которая используется только различными operator() и get_helper() методами, вы должны удалить его из класса, и вы можете распределить его статически в каждом operator().Также немного странно, что вы предлагаете отдельные operator() для 1, 2 и 3 измерений, в то время как код является общим для обработки любых измерений, а также что вы фактически требуете от пользователя * знайте , что из общедоступного интерфейса две операции запрещены, что тоже не очень удачно.

...