WW #18

[Egor Suvorov]

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

• Не компилируется из-за math.h и std::log10 одновременно — тут надо определиться, в каком пространстве имён хотите log10:

     In file included from tests/01_smoke_test.cpp:1:
  ../../../cpp19/karnaukhov.kirill/lab_18/include/format.hpp:53:41: error: no member named 'log10' in namespace 'std'; did you mean simply 'log10'?
          return static_cast<std::size_t>(std::log10((value > 0 ? value : -value) + 1) + 3);
                                          ^~~~~~~~~~
                                          log10
  /usr/lib/llvm-10/bin/../include/c++/v1/math.h:1019:1: note: 'log10' declared here
  log10(_A1 __lcpp_x) _NOEXCEPT {return ::log10((double)__lcpp_x);}
  ^
  1 error generated.
  make: *** [bin/01_smoke_test] Error 1
  01_smoke_test: failed compilation.
  

• Падает с bad_alloc на каких-то крайних случаях с числами. Ой не надо делать - от всего подряд в обобщённом коде. Да и log10 откровенно не внушает доверия.
• Откуда взялась константа MAX_SIZE = 25? Разные компиляторы поддерживают разные размеры целых чисел. Хотя бы assert поставили для приличия.
  • В частности, из-за этого у вас может в строчку добавиться полный мусор. Не UB, к счастью, но мусор (смотри первое замечание по стилю).

Стиль 1.5/3:

• Не проверяется, что to_chars успешно.
• back_insert_iterator тут категорически не нужен. Хватит +=, будет проще и читаемее. А в FormatHelper<bool> можно даже с тернарным оператором.
  • А ещё напоминаю про string_view, может пригодиться.
• В FormatHelper<T> для .format() misleading название константы MAX_SIZE. Это ни разу не MAX.
• Можно обойтись без member detection для FormatHelper<T>().estimate_size(). Подсказка: вы полностью контроллируете, что лежит внутри всех случаев FormatHelper.
  • Ну или хотя бы вспомогательный псевдоним сделайте.
• Для вектора
  • //special symbols — лучше явно перечислить, что за символы и пояснить, откуда взялась формула.
  • Лучше создать FormatHelper заранее.

Дополнительно по вашим тестам, на баллы не влияет:

• Не компилируется из-за нарушения ODR: is_formattable объявлена в разных единицах трансляции.
  • Посмотрите на ​https://github.com/evlinsky/cpp/commit/3ce057cbea7f9e2c2b609259a9cf45d883300f4a#diff-cfa302125750d623855e6553abe6a75d

[Egor Suvorov]

Бегло посмотрел код:

• Скобочки вокруг тернарного не нужны
• В целочисленных какой-то странный if: конструкция вида if (a) { b; } else assert(false); должна переписываться без if
• + 5; //5 -- sign and error (because of multiplying by log10) — плохое пояснение. Всё ещё неясно, откуда именно берётся ошибка и при чём тут умножение на log10.
• В варианте для вектора сейчас есть много-много перевыделений памяти в конструкторе. Это нехорошо, вся идея FormatHelper в том, чтобы лишний раз не перевыделять строчку, а сначала посчитать размер буфера, потом всё сделать. Я скорее предполагал, что создаётся vector<FormatHelper>.

[Egor Suvorov]

И, да, вместо проверки наличия деструктора (хоть и прикольно) я бы скорее добавил статическую константу IsInstantiated. Но тут на вкус и цвет — читаться, конечно, будет проще, зато можно её случайно забыть.

Наверное, лучше всего оставить как у вас и добавить псевдоним "можно ли отформатировать тип T" для була, чтобы было пояснение "чего это может деструктора не быть".

[Egor Suvorov]

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

Стиль 2.5/3:

• Неконсистентный пробел вокруг &: где-то const auto &x, где-то const auto& x.
• Комментарий стоит отформатировать так: // Special symbols: '{', '}', and ', ' for each element.
  • Аналогично следует добавить пробел в комментарии рядом с MAX_SIZE
• LIMIT_SIZE --> DEFAULT_ESTIMATED_SIZE.
• Не хватает noexcept в нескольких местах.

[Egor Suvorov]

Почти ура:

• special symbols --> Special symbols

Думаю, справитесь поправить :) 10/10.