Простая реализация массива без границ-проверка


Я готовлю себе на несколько собеседований. Это простой массив (с обработкой ошибок для краткости). Любой вход или предложения по стилю и использования шаблонов будут оценены.

    #include <iostream>
    template <class T>
    class Array {
    public:
      explicit Array(const int&);
      Array(const int&, const T&);
      Array(const Array&);
      virtual ~Array();
      T getVal(const int&) const;
      void setVal(const int&, const T&);
      T& operator[] (T);
      Array& operator= (const Array&);

      template <class U>
      friend std::ostream& operator<< (std::ostream&, const Array<U>&);

    private:
      T* arr;
      int size;
    };

    template <class T>
    Array<T>::Array(const int& init_size) {
      arr = new T[init_size];
      size = init_size;
    }

    template <class T>
    Array<T>::Array(const int& init_size, const T& init_value) {
      arr = new T[init_size];
      size = init_size;
      for (int i = 0; i < size; ++i) {
        arr[i] = init_value;
      }
    }

    template <class T>
    Array<T>::Array(const Array& original) {
      size = original.size;
      arr = new T[size];
      for (int i = 0; i < size; ++i) {
        arr[i] = original.arr[i];
      }
    }

    template <class T>
    Array<T>::~Array() {
      delete [] arr;
    }

    template <class T>
    T Array<T>::getVal(const int& index) const {
      return arr[index];
    }

    template <class T>
    void Array<T>::setVal(const int& index, const T& value) {
      arr[index] = value;
    }

    template <class T>
    T& Array<T>::operator[] (T index) {
      return arr[index];
    }

    template <class T>
    Array<T>& Array<T>::operator= (const Array& copy) {
      if (arr == copy.arr)
        return *this;
     size = copy.size;
      delete [] arr;
      arr = new T[size];
      for (int i = 0; i < size; ++i) {
        arr[i] = copy.arr[i];
      }
      return *this;
    }

    template <class T>
    std::ostream& operator<< (std::ostream& out, const Array<T>& self) {
      out << "[ ";
      for (int i = 0; i < self.size-1; ++i) {
        out << self.arr[i] << ", ";
      }
      out << self.arr[self.size-1] << " ]";
      return out;
    }


6977
7
задан 7 августа 2011 в 07:08 Источник Поделиться
Комментарии
4 ответа

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

template <class T>
Array<T>::Array(const int& init_size)
{
arr = new T[init_size]();
// ^^^^^ Add braces. on types it makes no difference.
// On POD it will force zero initialization.
size = init_size;
}

Используйте списки инициализации в конструкторах.

template <class T>
Array<T>::Array(const int& init_size)
: arr(new T[init_size]())
, size(init_size)
{}

Использовать скопировать и идиома подкачки для оператора присваивания (это автоматически обеспечивает сильную гарантию исключения).

template <class T>
Array<T>& Array<T>::operator= (const Array& copy)
{
if (arr == copy.arr) // This is the wrong test.
return *this; // Though it works as a side effect you will confuse the maintainer.

size = copy.size;
delete [] arr; // breaks the strong exception gurantee.
// What it the next line fails? Then you are left with an object
// in an inconsistent state. (arr points at de-allocated memory)
arr = new T[size];

for (int i = 0; i < size; ++i)
{
arr[i] = copy.arr[i];
}
return *this;
}

Если вы хотите сделать этот трудный путь, то это должно выглядеть так:

template <class T>
Array<T>& Array<T>::operator= (const Array& copy)
{
if (this == &copy) // Return quickly on assignment to self.
{ return *this;
}

// Do all operations that can generate an expception first.
// BUT DO NOT MODIFY THE OBJECT at this stage.
T* tmp = new T[size];
for (int i = 0; i < size; ++i)
{
tmp[i] = copy.arr[i];
}

// Now that you have finished all the dangerous work.
// Do the operations that change the object.
std::swap(tmp, arr);
size = copy.size;

// Finally tidy up
delete tmp; // Notice the swap above.

// Now you can return
return *this;
}

В качестве альтернативы используйте копию и идиома своп

template <class T>
Array<T>& Array<T>::operator=(Array const& rhs)
{
// 1 Copy
Array copy(rhs); // Use the copy constructor to make a safe copy.

// 2 Swap
swap(arr, copy.arr);
swap(size, copy.size);

} // 3 as the local copy goes out of scope it destroys the old array.

Обратите внимание, это может быть тоже оптимизирована:

Array<T>& Array<T>::operator=(Array rhs) // 1 implicit copy as parameter is passed by value
{
// 2 Swap
swap(arr, copy.arr);
swap(size, copy.size);

} // 3 as the local copy goes out of scope it destroys the old array.

Вы не нуждаетесь в методы get и set. На самом деле получить/установить явный признак плохого дизайна.

template <class T>
T Array<T>::getVal(const int& index) const {
return arr[index];
}

template <class T>
void Array<T>::setVal(const int& index, const T& value) {
arr[index] = value;
}

Просто удалить оба этих метода. Что вы хотите сделать, это вернуть ссылку на внутренний объект в перегрузке оператора[] (который вы делаете) бит индекс является целым числом, как правило.

template <class T>
T& Array<T>::operator[] (size_t index)
{ // ^^^^^^^ Changed type here.
return arr[index];
}

Вы также должны предоставить варианта для использования, когда Ваш массив как const.

template <class T>
T const& Array<T>::operator[] const (size_t index)
//^^^^^ Here ^^^^^ and here.
{
return arr[index];
}

Если вы предоставляете оператор вывода вы должны, как правило, симметричными и сообщите оператору ввода.

10
ответ дан 7 августа 2011 в 08:08 Источник Поделиться

@Мартин (в частности) уже дал некоторые отличные советы по целому ряду пунктов, но я думаю, что есть несколько, которые стоит упомянуть.

Прежде всего, вы использовали трижды-проклят массив новое выражение, чтобы выделить вашу память. Хотя это работает (на достаточно расплывчатое определение "работы"), он не идеален, по крайней мере на мой взгляд.

Я бы предпочел использовать ::оператор new для выделения "сырой" памяти, а затем, используя оператор new, чтобы скопировать объекты в массив.

Во-вторых, вы указали как явный конструктор для создания хранилища неинициализированным, и два параметра конструктора, чтобы создать массив, инициализированный с указанным значением. Я думаю, что я бы предпочел, чтобы сконденсировать два в один конструктор по умолчанию аргумент по первоначальной стоимости, так что вы можете никогда (даже случайно) создать массив неинициализированных данных.

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

template <class T>
Array<T>::Array(size_t init_size, const T& init_value) :
arr((T *)::operator new(init_size * sizeof(T))),
{
for (size = 0; size<init_size; ++size)
new(arr+size) T(init_value);
}

Обратите внимание, что вместо локальной переменной, чтобы пройти через массив и инициализировать значения, мы используем размер, так что если исключение не попал в петлю, размер будет содержать количество элементов мы успешно инициализирован. Конечно, мы должны модифицировать конструктор копирования, чтобы соответствовать:

template <class T>
Array<T>::Array(const Array &original) :
arr((T *)::operator new(original.size * sizeof(T))),
{
for (size = 0; size < original.size; ++size)
new(arr+size) T(original.arr[size]);
}

Тогда в нашем dtor, мы можем использовать это, чтобы уничтожить именно/только те объекты, которые были успешно построены. Чтобы соответствовать нормальной конвенций, мы уничтожаем тех, кто в обратном порядке создания:

template <class T>
Array<T>::~Array() {
for (size_t i=size+1; i>0; --i)
arr[i-1].~T();
::operator delete(arr);
}

Чтобы пойти с этим, вы (конечно) нужно изменить определение класса, чтобы соответствовать:

template <class T>
class Array {
T* arr;
size_t size;
public:
explicit Array(size_t, const T& init_value = T());
Array(const Array&);
~Array();
T& operator[] (size_t);
Array& operator= (Array);
};

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

Другая вещь, я думаю, что я бы сделал если бы я писал такой класс, как это можно было бы добавить достаточно для того, чтобы выступать в качестве контейнера, как это определено стандартом (определить итератор типа, типа value_type, начала() и конца() , возвращающие итераторы и т. д.)

3
ответ дан 10 августа 2011 в 03:08 Источник Поделиться

Видя, как вы предоставите оператора[], вы не должны принять оператора<< друг: вы можете быть уверены, что оператор[] будет подставляться, в любом случае (хотя вы должны предоставить константный вариант, как кто-то предложил).

Лично я бы использовать указатель уровня для arr для того, чтобы сделать его более безопасный в отношении исключений (с NDEBUG включен, он не должен вызвать каких-либо системных ресурсов). (Копия и замена фразеологизма работает для этого тоже.)

Кстати, я предполагаю, что ваш оператор[] принимает Т была опечатка, она должна занять реализация, как и почти все, что в настоящее время принимает тип int.

Я не вижу проблемы с вашей getVal и setVal функции-они делают ровно то же самое, что оператор[], а всего более многословный способ написания вещей (более обычной является на функцию, которая не выходит за рамки проверки и возвращает ссылку, но ничего страшного).

Я бы не назвал свой класс массива, как это, в моем понимании, ассоциируется с вещами во время компиляции, размер-не менее DynArray будет точнее.

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

1
ответ дан 14 августа 2011 в 05:08 Источник Поделиться

Почему вы проходите целых чисел как const ссылки - это будет просто замедлить ваш код, и можно запутать читателя, ибо никакой пользы.

Я подзабыл на шаблонах, так что я могу ошибаться, но не

  Array(const Array&);

Позволит вам попробовать построить массив из массива - не должно быть

  Array(const Array<T>&);

Аналогично для оператора присваивания.

Просто пара мыслей с верхней части моей головы в воскресный вечер.

0
ответ дан 7 августа 2011 в 08:08 Источник Поделиться