WW #18

[Egor Suvorov]

Корректность 2/7:

• Не работает с некоторыми пограничными значениями целых чисел.
• Слишком грубые оценки на длину була.
• Оценка на вектор идёт снизу => есть лишние перевыделения памяти.

Стиль 0.5/3:

• Константы true_str/false_str/esimated_size лесом.
  • Скобочки вокруг тернарного оператора не нужны.
• Не проверяется, что to_chars завершился успешно.
  • Не используйте расширение GCC/Clang под названием VLA, позволяющее создавать массивы на стеке произвольного размера.
• formattable не нужны аргументы. ... возникают только там, где надо сделать перегрузку с минимальным весом — у вас тут вообще хватит константы.
• Решение FormatHelper<typename, bool> очень плохо расширяется. Например, если попросить добавить вас ещё один вид метода format(), то придётся костылить и усложнять одну из двух специализаций.
  • Например, уже сейчас требуется лишний комментарий FormatHelper<T, false> // For types with format method. А проверка наличия этого метода спрятана глубоко в метод formattable.
  • Посмотрите с практики, как можно вырезать специализации по SFINAE. Будет гораздо симметричнее.
• В векторе у вас FormatHelper создаётся дважды.
  • Не надо push_back, особенно с , вместо ; (это прям вообще очень плохо выглядит). Лучше +=, да.
• Константа esimated_size для .format() не соответствует своему названию.

[Egor Suvorov]

Корректность 4/7:

• В FormatHelper для вектора лишние перевыделения памяти в конструкторе. Много перевыделений.
• Неверная оценка размера для пустых векторов прямо на smoke-тесте. Ваше счастье, что тут UB не случилось (как минимум из-за переполнения). В следующем ответе поясните, пожалуйста, в чём была проблема.

Стиль 1.25/3:

• std::accumulate у вас вызывается для T=int, а не size_t.
• Не хватает слов final
• Detecter --> Detector
• // BOOL Specialization — комментарий, имхо, бесполезный, полностью дублирует следующие две строки и ничего не добавляет к коду. Заодно появляется возможность поправить код и не поправить комментарий, что плохо. Аналогично с остальными комментариями про специализации.
• Неаккуратные оценки на длину була. Посмотрите, как этот estimate_size используется.
• Неясно, откуда взялась константа 2 в специализации для целых чисел. Нужно пояснение-комментарий в коде — вот тут комментарий реально по делу.
• Непонятен комментарий formatted += '{' ?
• max_chars_count и is_formattable большими буквами, пожалуйста — это коснтанты.
• Вместо buffer, buffer + some_constant_which_is_hopefully_synchronized_with_the_line_above лучше std::begin(), std::end() из <iterator>. Тогда не надо будет глазами проверять, что корректно указан размер буфера в двух строчках.

[Egor Suvorov]

Не компилируется:

In file included from tests/01_smoke_test.cpp:1:
../../../cpp19/brilliantov.kirill/lab_18/include/format.hpp:98:26: error: use of undeclared identifier 'data'; did you mean 'std::data'?
    return 2 /* {} */ + (data.empty() ? 0 : 2 * (data.size() - 1)) /*delims*/ + elements_eval;
                         ^~~~
                         std::data
/usr/lib/llvm-10/bin/../include/c++/v1/iterator:1982:6: note: 'std::data' declared here
auto data(_Cont& __c)
     ^
In file included from tests/01_smoke_test.cpp:1:
../../../cpp19/brilliantov.kirill/lab_18/include/format.hpp:98:26: error: reference to overloaded function could not be resolved; did you mean to call it?
    return 2 /* {} */ + (data.empty() ? 0 : 2 * (data.size() - 1)) /*delims*/ + elements_eval;
                         ^~~~
/usr/lib/llvm-10/bin/../include/c++/v1/iterator:1982:6: note: possible target for call
auto data(_Cont& __c)
     ^
2 errors generated.

[Egor Suvorov]

Корректность 7/7.

Стиль 2.5/3:

• MAX_CHARS_COUNT в format.hpp неверно аргументирован. to_chars не выводит null-terminated string.
• 0ULL — лучше static_cast<std::size_t>