Замечания по классу CMyString

[alexey-malov]

    CMyString const SubString(unsigned start, unsigned length = UINT_MAX)const;

Аргументы должны иметь тип size_t, иначе в 64-битной конфигурации, часть строки за пределами 4Гб будет недоступна

[alexey-malov]

    void operator =(CMyString const &other);
    void operator =(CMyString &&other);
    void operator +=(CMyString const &other);
    char& operator [](size_t index);
    char operator [](size_t index) const;

Оператор = и += должны возвращать тип CMyString&, чтобы можно было записывать выражения вроде:

if ((scheme = url.SubString(0, 7)) == "http://")
{
     // this is an http URL
}

Константный оператор должен возвращать константную ссылку, а не значение, чтобы можно было написать что-то вроде:

printf("%s", &someStr[0]);

Если вернуть значение, то вернется адрес одиночной копии символа строки, по которому уже не будет располагаться ASCIIZ строка

[alexey-malov]

    size_t GetNewSize(size_t requiredSize);

Судя по имени, метод не изменяет состояние объекта. В этом случае он должен быть константным

[alexey-malov]

CMyString operator +(CMyString str1, CMyString const &str2);

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

CMyString(size_t capacity, const char * pString, size_t length);

в котором выделить память под длину результирующей строки. Либо использовать SetSize

[alexey-malov]

SetSize лучше переименовать в SetCapacity (вместимость), а переменную m_allocatedSize - в m_capacity.

m_dataSize лучше переименовать в m_length

[alexey-malov]

size_t CMyString::GetNewSize(size_t requiredSize)
{
    size_t newSize = m_allocatedSize;
    while (requiredSize > newSize)
    {
        newSize *= 2;
    }
    return newSize;
}

Можно обойтись без цикла, если воспользоваться примерно таким кодом:

if (requiredSize > m_capacity)
{
    return std::max(requiredSize, m_capacity * 2);
}
return m_capacity;

либо return (...) ? ... : ... ;

[alexey-malov]

void CMyString::operator =(CMyString &&other)
{
    m_dataSize = other.m_dataSize;
    m_allocatedSize = other.m_allocatedSize;
    m_pData = move(other.m_pData);
}

Экземпляр other остается без массива символов, но со старой длиной и вместимостью. После перемещения объект должен оставаться в состоянии, позволяющем его безопасно разрушить или переместить в другой объект. Вероятно, придется допустить, что capacity может стать равным nullptr (после перемещения). В этом случае надо добавить обработку этого состояния в других методах и операторах

[alexey-malov]

CMyString::CMyString(std::string const &stlString) :
m_pData(make_unique<char[]>(m_allocatedSize))
{
    *this = stlString.c_str();
}

Строка stlString может внутри себя содержать символы с нулевым кодом (в середине), которые будут потеряны. Нужно использовать длину строки, возвращаемую методом length()

[alexey-malov]

char const *CMyString::GetStringData() const
{
    return m_pData.get();
}

По условию метод GetStringData должен возвращать asciiz строку и в том случае, когда строка пустая. Пустой строка может стать и после перемещения данных из нее. Можно решить эту проблему, если хранить статическую переменную класса типа const char, содержащую символ с нулевым кодом

[alexey-malov]

void CMyString::operator =(CMyString const &str2)
{
    m_dataSize = str2.m_dataSize;
    SetSize(m_dataSize + 1);
    memcpy(m_pData.get(), str2.m_pData.get(), m_dataSize + 1);
}

Лучше поставить защиту от самоприсваивания, чтобы не полагаться на то, что memcpy достаточно умен, чтобы не копировать данные сами в себя. Кроме того, требование к функции memcpy такое:

To avoid overflows, the size of the arrays pointed to by both the destination and source parameters, shall be at least num bytes, and should not overlap (for overlapping memory blocks, memmove is a safer approach).

[alexey-malov]

void CMyString::SetSize(size_t requiredSize)
{
    size_t newSize = GetNewSize(requiredSize);
    if (m_allocatedSize < newSize)
    {
        m_allocatedSize = GetNewSize(requiredSize);
        auto pNewData = make_unique<char[]>(m_allocatedSize);
        memcpy(pNewData.get(), m_pData.get(), m_dataSize + 1);
        m_pData = move(pNewData);
    }
}

При возникновении исключения в make_unique объект останется в рассогласованном состоянии (m_allocatedSize не будет соответствовать реальной вместимости строки). Надо модификацию состояния объекта отложить напотом (после того, как операции, способные бросить исключение, выполнятся).

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

[alexey-malov]

[alexey-malov]

CMyString &CMyString::operator =(CMyString &&other)
{
    m_length = other.m_length;
    m_capacity = other.m_capacity;
    m_pData = move(other.m_pData);

    other.m_length = 0;
    other.m_capacity = 0;

    return *this;
}

При самоперемещении строка обнулится

[alexey-malov]

bool operator <=(CMyString const &str1, CMyString const &str2)
{
    return ((str1 < str2) || (str1 == str2));
}

bool operator >=(CMyString const &str1, CMyString const &str2)
{
    return ((str1 > str2) || (str1 == str2));
}

Желательно определить результат (с учетом длины строк) сравнения за один проход (можно сделать функцию compare возвращающую int (-1, 0, +1), на основе которой реализовать эти операторы

[alexey-malov]

    if (!m_pData)
    {
        m_pData = make_unique<char[]>('\0');
        m_capacity = 1;
    }   

Тут ты выделяешь в куче блок размером 0 байт

[alexey-malov]

    size_t newSize = GetNewSize(requiredSize);
    if (m_capacity < newSize)
    {
        auto pNewData = make_unique<char[]>(m_capacity);
        m_capacity = newSize;
        memcpy(pNewData.get(), m_pData.get(), m_length + 1);
        m_pData = move(pNewData);
    }

Тут выделили память под m_capacity (меньше, чем newSize) А потом считаем, что выделили newSize

[alexey-malov]

    class iterator
    {
    public:
        iterator(CMyString const *pString, char *pChar);
    public:
        iterator &operator ++();
        iterator operator ++(int);
        iterator &operator --();
        iterator operator --(int);
        iterator operator +(int offset) const;
        iterator operator -(int offset) const;
        int operator -(iterator const &it2) const;
        char const &operator *() const;
        char &operator *();
        char const& operator [](int index) const;
        char &operator [](int index);
        bool operator ==(iterator const &it2) const;
        bool operator !=(iterator const &it2) const;
    private:
        CMyString const *m_pString;
        char *m_pChar;
    };

const interator и const_interator
разные сущности. const_iterator - итератор, который не позволяет изменять элементы контейнера, но делает само итерирование доступным. const iterator - это итератор, который позволяет измнять элементы контейнера, но не может указывать на другой элемент контейнера

[alexey-malov]

Для разницы итераторов следует использовать ptrdiff_t

[alexey-malov]

---

[alexey-malov]

    if (!m_pData)
    {
        m_pData = make_unique<char[]>(15);
        m_capacity = 15;
        *m_pData.get() = '\0';
    }   
    size_t newSize = GetNewSize(requiredSize);
    if (m_capacity < newSize)
    {
        auto pNewData = make_unique<char[]>(newSize);
        memcpy(pNewData.get(), m_pData.get(), m_length + 1);
        m_pData = move(pNewData);
        m_capacity = newSize;
    }

Устранить избыточность при выделении памяти (1 выделение вместо 2-х)

[alexey-malov]

Реверсивный итератор лучше объявить через typedef с использованием адаптера std::reverse_iterator

[alexey-malov]

285 100 20 24 30 20 = 479