﻿# Старайтесь не писать строки кода, которые не помещаются на экран

Встретили баг, который одновременно демонстрирует, как "код\-колбаса" притягивает ошибку, эффект последней строки и проблему неудачного copy\-paste\.

![1316_long_line_ru/image1.png](https://import.viva64.com/docx/blog/1316_long_line_ru/image1.png)

Мы не только реализуем в PVS\-Studio новые диагностические правила, но и постепенно дорабатываем старые\. Например, недавно в C\#\-анализаторе мы улучшили одну из самых старых диагностик — V3001, научив её корректнее определять случаи, когда дополнительные скобки ни на что не влияют\. Результат — выявление новых багов, с одним из которых мы сейчас вас познакомим\.

Ошибка найдена в проекте Space Engineers, который входит в базу проектов для регрессионного тестирования нашего анализатора\. Мы используем старую зафиксированную версию проекта, так как нам нужно анализировать отличие работы анализатора на одном и том же коде\. Но если заглянуть в свежую версию исходного кода, то можно обнаружить, что ошибка по\-прежнему присутствует\. См\. функцию `Contains` в [BoundingBox\.cs](https://github.com/KeenSoftwareHouse/SpaceEngineers/blob/54f2f0f3169cda687a25a438097902a43bdfa603/Sources/VRage.Math/BoundingBox.cs#L816C32-L816C40)\.

Видите ошибку? Думаю, нет\.

А всё почему? Потому, что не надо писать такие длинные нечитаемые строки кода\. В них очень легко допустить ошибку, что, собственно, и произошло\. Отформатирую код\.

```cpp
public ContainmentType Contains(BoundingSphere sphere)
{
  Vector3 result1;
  Vector3.Clamp(ref sphere.Center, ref this.Min, ref this.Max, out result1);
  float result2;
  Vector3.DistanceSquared(ref sphere.Center, ref result1, out result2);
  float num = sphere.Radius;
  if ((double)result2 > (double)num * (double)num)
    return ContainmentType.Disjoint;
  return (double)this.Min.X + (double)num > (double)sphere.Center.X ||
         (double)sphere.Center.X > (double)this.Max.X - (double)num ||
         ((double)this.Max.X - (double)this.Min.X <= (double)num ||
          (double)this.Min.Y + (double)num > (double)sphere.Center.Y) ||
         ((double)sphere.Center.Y > (double)this.Max.Y - (double)num ||
          (double)this.Max.Y - (double)this.Min.Y <= (double)num ||
         ((double)this.Min.Z + (double)num > (double)sphere.Center.Z ||
          (double)sphere.Center.Z > (double)this.Max.Z - (double)num)) ||
         (double)this.Max.X - (double)this.Min.X <= (double)num ?
           ContainmentType.Intersects : ContainmentType.Contains;
}
```

Стало лучше, но всё равно надо постараться, чтобы заметить ошибку\. Обратите внимание на последнюю строчку логического условия:

```cpp
(double)this.Max.X - (double)this.Min.X <= (double)num
```

Она повторяет третью строчку\. Условие выше заключено в дополнительные скобки, но они ни на что не влияют, так как все проверки объединяются с помощью оператора ИЛИ\.

На самом деле в последнем случае должна проверяться координата Z:

```cpp
(double)this.Max.Z - (double)this.Min.Z <= (double)num
```

Анализатор PVS\-Studio замечает это и выдаёт предупреждение: V3001 There are identical sub\-expressions '\(double\)this\.Max\.X \- \(double\)this\.Min\.X <\= \(double\)num' to the left and to the right of the '\|\|' operator\.

Хороший пример, когда статический анализатор дополняет обзор кода, в процессе которого сложно заметить опечатку в такой длинной строке\. Я называю это "кодом\-колбасой" и уже писал [заметку](https://pvs-studio.ru/ru/blog/posts/cpp/1230/) о том, как он притягивает баги\.

Здесь проявил себя и "[эффект последней строки](https://pvs-studio.ru/ru/blog/posts/cpp/0260/)"\. Опечатки чаще всего появляются в конце однотипных фрагментов кода\. Здесь, правда, нельзя говорить про строки, так как строка одна\. Но суть та же: ошибка допущена в последней части длинного выражения, состоящего из схожих блоков\.

Ошибка возникла из\-за copy\-paste\. Подвыражения, видимо, писались с помощью копирования предыдущих, и в одном из мест в копию не были внесены нужные изменения\. Однако это ещё не всё\. Вся эта строка с ошибкой ещё раз была размножена копированием, и её можно наблюдать в соседней функции [Contains](https://github.com/KeenSoftwareHouse/SpaceEngineers/blob/54f2f0f3169cda687a25a438097902a43bdfa603/Sources/VRage.Math/BoundingBox.cs#L832C21-L832C29) несколькими строками ниже:

```cpp
public void Contains(ref BoundingSphere sphere, out ContainmentType result)
{
  ....
  if ((double)result2 > (double)num * (double)num)
    result = ContainmentType.Disjoint;
  else
    result = (double)this.Min.X + (double)num > (double)sphere.Center.X ||
             (double)sphere.Center.X > (double)this.Max.X - (double)num ||
             ((double)this.Max.X - (double)this.Min.X <= (double)num ||
              (double)this.Min.Y + (double)num > (double)sphere.Center.Y) ||
             ((double)sphere.Center.Y > (double)this.Max.Y - (double)num ||
              (double)this.Max.Y - (double)this.Min.Y <= (double)num ||
             ((double)this.Min.Z + (double)num > (double)sphere.Center.Z ||
              (double)sphere.Center.Z > (double)this.Max.Z - (double)num)) ||
             (double)this.Max.X - (double)this.Min.X <= (double)num ?
               ContainmentType.Intersects : ContainmentType.Contains;
}
```

Всё то же самое, и анализатор указал на вторую ошибку точно таким же предупреждением\.

**Заключение**

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

1. [Форматированию кода таблицей](https://pvs-studio.ru/ru/blog/terms/7003/)\.
1. Вынесению единообразного кода в функции\.
1. Удалению лишних операций\. Например, вместо того, чтобы везде выполнять приведение типа `(double)num` можно было сразу объявить переменную `num` как `double`\.
1. Регулярному использованию статического анализатора [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) для дополнительного контроля\.