Dzzirt / OOP

0 stars 0 forks source link

Замечания по CMyString #18

Open alexey-malov opened 8 years ago

alexey-malov commented 8 years ago

Конструктор перемещения надо сделать не выбрасывающим исключений. Например, для всех пустых строк использовать массив из одного символа (статический). Указатель на данные хранить нулевой. Минимизировать количество мест в классе и его друзьях, напрямую работающих с указателями. Использовать для этих целей приватные методы. Чтобы случайно не передать нулевой указатель туда, где его использовать нельзя.

alexey-malov commented 8 years ago

Методы, возвращающие const CMyString должны возвращать CMyString, чтобы компилятор мог использовать перемещение временных объектов

alexey-malov commented 8 years ago

удостовериться, что переполнение при сложении внутри substring выдает ожидаемые подстроки

alexey-malov commented 8 years ago

Clear не должен бросать исключений

alexey-malov commented 8 years ago

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

alexey-malov commented 8 years ago

перемещающий оператор присваивания не должен бросать исключений (при донор останется в невалидном состоянии) даже можно написать у него noexcept

alexey-malov commented 8 years ago

копирующий оператор присваивания идиоматичнее реализовать через tmp copy construction+swap

alexey-malov commented 8 years ago

оператор < должен позволять определить лексикографический порядок строк.

слова разной длины должны сравниваться по длине меншего слова. Если контент равен, то более длинное слово следует за более коротким (которое является его префиксом)

alexey-malov commented 8 years ago

оператор > может приводить к неопределенному поведению и все остальные тоже

alexey-malov commented 8 years ago

операторы сравнения лучше реализовать так, чтобы сравнение выполнялось за один проход. РАсшифровка: <= не реализовывать через < || ==

alexey-malov commented 8 years ago

Рекомендация: сделать int Compare(), в которой разместить проверку длин и memcmp. Операторы реализовать через эту функцию

alexey-malov commented 8 years ago

в [] добавить assert для проверки аргументов хотя бы в отладочной конфигурации

alexey-malov commented 8 years ago

оператор >> реализован не по канону. Он должен заменять правый аргумент, а не аппендить В std sting он реализован именно так

Dzzirt commented 8 years ago

Fixed

alexey-malov commented 8 years ago

вызов SafeDelete всегда связан с модификацией m_first на некоторое значение Зачем делать m_first равным m_emptyStr? Получать указатель можно GetStringData() в том числе и внутри класса. Тогда в SafeDelete проверка m_emptyStr не нужна

alexey-malov commented 8 years ago

Substr у перемещенной строки обратится к нулевому указателю Проверить это в тестах

alexey-malov commented 8 years ago

Compare() реализован неправильно, для срок с одинаковым префиксом не может вернуть отрицательное значение Если левая строка короче правой и является полным префиксом правой, результат будет "строки равны" В тестах воспроизвести эту ситуацию