Замечания по CMyArray

[alexey-malov]

• [ ] Не надо #include-ить .cpp файл в тестах. Перенесите в заголовочный файл тела шаблонных методов.

[alexey-malov]

template <typename T>
CMyArray<T>::CMyArray(T* arr, size_t size, size_t capacity)
    : CMyArray(capacity)
{
    m_size = size;
    CMyArrayIterator<T> currPtr(m_arr);
    CMyArrayIterator<T> endPtr(arr + size);
    try
    {
        for (CMyArrayIterator<T> otherPtr(arr); otherPtr != endPtr; ++currPtr, ++otherPtr)
        {
            new (currPtr.m_item) T(*otherPtr);
        }
    }
    catch (...)
    {
        DestroyArr(m_arr, currPtr.m_item);
    }
}

• [ ] Почему arr - указатель на неконстантные данные? Разве конструктор модифицирует исходный массив?

[alexey-malov]

template <typename T>
void CMyArray<T>::Clear()
{
    CMyArray temp;
    std::swap(m_arr, temp.m_arr);
    std::swap(m_size, temp.m_size);
    std::swap(m_capacity, temp.m_capacity);
}

• [ ] Clear не должен выделять память. Но он выделяет, т.к. конструктор по умолчанию создаёт массив вместимостью 1

[alexey-malov]

template <typename T>
void CMyArray<T>::Push(const T& item)
{
    if (m_size < m_capacity)
    {
        m_arr[m_size] = item; // (1)
        ++m_size;
    }
    else
    {
        CMyArray temp(m_arr, m_size, m_capacity + CAPACITY_PUSH_STEP);
        std::swap(m_arr, temp.m_arr);
        std::swap(m_size, temp.m_size);
        std::swap(m_capacity, temp.m_capacity);

        m_arr[m_size] = item; // (2)
        ++m_size;
    }
}

• [ ] В строках (1) и (2) неопределённое поведение из-за обращения к непроинициализированным объектам. Надо вызывать placement new

[alexey-malov]

template <typename T>
void CMyArray<T>::Resize(size_t size)
{
    if (size <= m_capacity)
    {
        m_size = size; // (1)
    }
    else
    {
        size_t oldSize = m_size;
        *this = CMyArray(m_arr, size, size); // (2)
        for (size_t offset = oldSize; offset < size; ++offset)
        {
            m_arr[offset] = T();
        }
    }
}

• [ ] В строке (1) мы просто перезаписываем длину, но не разрушаем ставшие недоступными объекты.

• [ ] В строке (2) во временный массив будут копироваться элементы, выходящие за пределы исходного массива - будет неопределённое поведение.

[alexey-malov]

template <typename T>
void CMyArray<T>::operator=(const CMyArray& other)
{
    if (other.m_size <= m_capacity)
    {
        memcpy(m_arr, other.m_arr, other.m_size); // (1)
        m_size = other.m_size;  // (2)
    }
    else
    {
        CMyArray temp(other);
        std::swap(m_arr, temp.m_arr);
        std::swap(m_size, temp.m_size);
        std::swap(m_capacity, temp.m_capacity);
    }

• [ ] В строке (1) происходит побитовое копирование объектов, что в общем случае недопустимо
• [ ] В строке (2) перезаписывается длина, но не разрушаются ранее существовавшие элементы массива

[alexey-malov]

template <typename T>
void CMyArray<T>::operator=(CMyArray&& other)
{
    if (other.m_size <= m_capacity)
    {
        memcpy(m_arr, other.m_arr, other.m_size); // (1)
        m_size = other.m_size;  // (2)
    }
    else
    {
        CMyArray temp(other);
        std::swap(m_arr, temp.m_arr);
        std::swap(m_size, temp.m_size);
        std::swap(m_capacity, temp.m_capacity);
    }
}

• [ ] Замечания аналогичны копирующему присваиванию

• [ ] Перемещающий оператор присваивания должен перемещать элементы из одного массива в другой, а не копировать. Выполняться он должен за O(1)

[alexey-malov]

• [ ] Должна иметься возможность создать константный итератор из неконстантного