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

[alexey-malov]

1>C:\teaching\oop-labs\voronin\OOP\lab_2\ExpandTemplate\ExpandTemplate.cpp(105,50): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data
1>C:\teaching\oop-labs\voronin\OOP\lab_2\ExpandTemplate\ExpandTemplate.cpp(149,44): warning C4267: '+=': conversion from 'size_t' to 'int', possible loss of data

• [ ] Для индексации элементов строк и векторов в STL используется тип size_t, а не int. На 64 разрядных платформах его размер может быть больше размера int. В результате на длинных строках ваша программа может работать некорректно

Если хотите хранить знаковый тип, размер которого совпадает с size_t - используйте ptrdiff_t

[alexey-malov]

>ExpandTemplate.exe input.txt output.txt мир хлеб

• [ ] Программа падает при таких входных данных

[alexey-malov]

• [ ] В коде много приведений типа вроде int(ch). Используйте тип данных, который бы минимизировал необходимость в приведении типов

[alexey-malov]

https://github.com/voronindim/OOP/blob/2fc9235f2775243c9241b8f59ebf17af569ef05c/lab_2/ExpandTemplate/ExpandTemplate.cpp#L81-L96

• [ ] Использование const_cast для снятия константности- скорее всего ошибка. В функцию передаются адреса строк, находящихся в ключах контейнера map. Ключи контейнера map должны быть константными - изменять их значение нельзя, иначе это приведет к нарушению целостности контейнера и неопределенному поведению. Рассмотрите следующие альтернативы указателям:
• std::string_view
• std::const_reference_wrapper (создается при помощи функции cref)

[alexey-malov]

https://github.com/voronindim/OOP/blob/2fc9235f2775243c9241b8f59ebf17af569ef05c/lab_2/ExpandTemplate/ExpandTemplate.cpp#L133-L154

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

Можно воспользоваться structure bindings (фича из C++17) для разбиения пары отдельные фрагменты:

    string resultStr;
    int pos = 0;
    for (auto& [startSubstring, stringToReplace] : substrings)

• [ ] Лучше воспользоваться версией append, позволяющей добавить к строке часть другой строки. Это позволит избавиться от извлечения подстроки, приводящего к выделению памяти в куче.

• [ ] Если предположить, что в шаблоне для поиска может быть много placeholder-ов, то лучше возвращать из SearchSubstrings не map<int, string>, а нечто более компактное: vector<tuple<size_t, string_view, string_view>>. Причины:

• Вам не нужна древовидная структура для поиска - обход выполняется последовательно

• Данные в vector хранятся последовательно, что лучше для кэша процессора.

• string_view не копирует строки, а лишь ссылается на ключи и значения, которые лежат себе преспокойно в params. Каждый string_view занимает 16 байт на 64-битной платформе. Правда, в структуру Bohr потребуется положить еще данные о строке-заменителе.

https://github.com/voronindim/OOP/blob/2fc9235f2775243c9241b8f59ebf17af569ef05c/lab_2/ExpandTemplate/ExpandTemplate.cpp#L148

• [ ] технически find в map-е может вернуть пустой итератор end, который разыменовывать нельзя. На деле можно ускорить, избежав поиска строки-заменителя в шаблоне, если

[alexey-malov]

https://github.com/voronindim/OOP/blob/2fc9235f2775243c9241b8f59ebf17af569ef05c/lab_2/ExpandTemplate/ExpandTemplate.cpp#L98-L108

• [ ] Имя функции, скорее всего, неудачное. Что она проверяет? Либо декомпозиция выполнена неудачно

[alexey-malov]

• [ ] Некоторые функции, работающие с коллекцией BohrVertex либо принимающие BohrVertex должны принимать коллекции или сами BohrVertes по константной ссылке, если они не изменяют их. Без этого трудно анализировать работу функций, что они меняют, а что не меняют

[alexey-malov]

• [ ] Есть ощущение, что не все сценарии использования функции ExpandTemplate были покрыты тестами. Например, сходу не заметил, что как программа работает с пустыми строками или строками, имеющими один и тот же префикс.