Почему обзоры кода — это хорошо, но недостаточно

[Duny]

Похоже, в условии пропущено отрицание:
if (input[i + 1] != '+' && input[i + 1] != '-' &&
(input[i + 1] >= '0' && input[i] <= '9')) // <--
return false;

!(input[i + 1] >= '0' && input[i+1] <= '9')

[putnik]

обзоры кода

Только с третьего раза понял о чём речь. И вроде перевод полностью корректный, но осознал, что так вообще никто вокруг меня не говорит

[tenzink]

Вот тут if конечно забавен — он никогда не сработает. Это типичная ошибка при использовании unsigned типов. Проверка призвана ловить строки из одних пробелов, но j улетает в небеса (-1).

PVS-Studio ловит такое?

  std::size_t i = 0, j = input.length() - 1;

  // Handling whitespaces
  while (i < input.length() && input[i] == ' ')
    i++;
  while (input[j] == ' ')
    j--;

  if (i > j)
    return false;

[Andrey2008]

Такое пока нет. ;) Но, спасибо.

[skymorp]

Еще забавный порядок действий:

  if (is_binary_notation(input) ||
      is_hex_notation(input) ||
      is_octal_notation(input)) {
    return true;
  }

  if (input.empty()) {
    return false;
  }

Сначала выполнили кучу действий и только потом проверили пустая строка или нет.

[lamerok]

Честно говоря, до ревью такой код не должен доходить вообще, что это за "ненормальное программирование"? У ребят вообще стандарт кодирования был? Сколько тут цикломатическая сложность? Даже смотреть бы код не стали бы, сразу на переработку… и искать тогда ничего не надо было бы в этой лапше.

[petropavel]

Это, безусловно, главное. Но даже если смотреть, утирая кровавые слёзы, чтоб, как попросили "найти ошибку", то их там же полно:

• цикл удаления пробелов с конца проскакивает начало строки
• dot_or_exp не ставится в true на '.'
• проверка на "last character" должна быть i + 1 >= input.length()
• и вообще-то должна быть проверка на предпоследний символ, чтоб можно было смотреть input[i+1]
• то же самое ниже, в проверке на 'e'
• 1e+ проверку пройдёт, 1e3e4,1e6figvam8, 1foo2.4 — тоже

Главное, конечно, не ошибки, а сам подход в топку.

[xammi-1]

Мне казалось "ревью кода" стало общепринятым термином. Полез в английскую версию и только так понял, что "обзор кода"="code review"

[GCU]

Можно ли считать ошибкой что любая каша из цифр и знаков плюс минус считается "валидным числом"?
Про строку из одних пробелов писали выше — по логике там вообще вылет будет.

[ad1Dima]

Возможно там никогда не попадает такое значение в функцию

UPD даже скорее всего не приходит. Это часть парсера, так что пробелы должны быть уже отсечены где-то выше.

[Andrey2008]

Итак, в комментариях к этой статье люди писали, что там есть и другие места, которые по всей видимости являются ошибками. И что вообще код написан сложно и неаккуратно. Очень рад, что данная тема вызвала интерес. Поэтому в продолжение предлагаю публикацию «Почему важно проводить статический анализ открытых библиотек, которые вы добавляете в свой проект» :). habr.com/ru/company/pvs-studio/blog/520498