Николай Миронов

16 Дек 2020

Теги:

#CSharp

Об ошибках в коде QuantConnect Lean

16 Дек 2020

Автор: Николай Миронов

Случайности не случайны
Передача параметров ссылочного типа
Освобождаем ресурсы
Вещественные числа
Оператор switch
Оператор if
Типичные ошибки для человека
Вывод

В данной статье рассматриваются ошибки в проекте с открытым исходным кодом, найденные с помощью статического анализатора. Говорится о некоторых простых вещах, которые могут помочь избежать их появления. Например, используя синтаксические конструкции языка начиная с C# 8.0. Надеюсь, что будет интересно. Приятного прочтения.

QuantConnect Lean - это алгоритмический торговый движок с открытым исходным кодом, созданный для легкого исследования стратегий, бэктестинга и живой торговли. Совместим с Windows, Linux и macOS. Интегрируется с распространенными поставщиками данных и брокерскими компаниями для быстрого развертывания алгоритмических торговых стратегий.

Проверка была осуществлена с помощью статического анализатора PVS-Studio. PVS-Studio - это инструмент для выявления ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках С, C++, C# и Java на Windows, Linux и macOS.

Случайности не случайны

public virtual DateTime NextDate(....)
{
  ....
  // both are valid dates, so chose one randomly
  if (   IsWithinRange(nextDayOfWeek, minDateTime, maxDateTime) 
      && IsWithinRange(previousDayOfWeek, minDateTime, maxDateTime))
  {
    return _random.Next(0, 1) == 0  // <=
           ? previousDayOfWeek
           : nextDayOfWeek;
  }
  ....
}

V3022 Expression '_random.Next(0, 1) == 0' is always true. RandomValueGenerator.cs 142

Смысл был в том, что выбирается либо одно, либо другое значение с 50% вероятностью. Однако в этом случае метод Next всегда будет возвращать 0.

Это происходит потому, что в диапазон значений не включается второй аргумент. То есть значение, которое метод может вернуть, будет находиться в диапазоне [0,1). Исправим это:

public virtual DateTime NextDate(....)
{
  ....
  // both are valid dates, so chose one randomly
  if (   IsWithinRange(nextDayOfWeek, minDateTime, maxDateTime) 
      && IsWithinRange(previousDayOfWeek, minDateTime, maxDateTime))
  {
    return _random.Next(0, 2) == 0
           ? previousDayOfWeek
           : nextDayOfWeek;
  }
  ....
}

Передача параметров ссылочного типа

Пример

/// <summary>
/// Copy contents of the portfolio collection to a new destination.
/// </summary>
/// <remarks>
/// IDictionary implementation calling the underlying Securities collection
/// </remarks>
/// <param name="array">Destination array</param>
/// <param name="index">Position in array to start copying</param>
public void CopyTo(KeyValuePair<Symbol, SecurityHolding>[] array, int index)
{
  array = new KeyValuePair<Symbol, SecurityHolding>[Securities.Count];
  var i = 0;
  foreach (var asset in Securities)
  {
    if (i >= index)
    {
      array[i] = new KeyValuePair<Symbol,SecurityHolding>(asset.Key,
                                                          asset.Value.Holdings);
    }
    i++;
  }
}

V3061 Parameter 'array' is always rewritten in method body before being used. SecurityPortfolioManager.cs 192

Метод принимает коллекцию и сразу же перезаписывает ее значение. Согласитесь, что это выглядит достаточно подозрительно. Поэтому давайте постараемся понять, что должен делать этот метод.

По комментарию и названию метода становится понятно, что в переданный массив должен скопироваться какой-то другой. Однако этого не произойдет, и значение у array вне текущего метода останется неизмененным.

Это происходит из-за того, что аргумент array будет передан в метод по значению, а не по ссылке. Таким образом, после выполнения операции присвоения ссылку на новый объект будет хранить переменная array, доступная внутри метода. Значение аргумента, переданного в метод, останется неизменным. Чтобы это исправить, необходимо передать аргумент ссылочного типа по ссылке:

public void CopyTo(out KeyValuePair<Symbol, SecurityHolding>[] array, // <=
                   int index)
{
  array = new KeyValuePair<Symbol, SecurityHolding>[Securities.Count];
  ....
}

Так как мы, безусловно, записываем в методе новый массив, то используем модификатор out вместо ref. Это сразу говорит о том, что переменной внутри будет присвоено значение.

Кстати, этот случай пополняет коллекцию, которую собирает мой коллега Андрей Карпов и про которую можно узнать из статьи "Начало коллекционирования ошибок в функциях копирования".

Освобождаем ресурсы

public static string ToSHA256(this string data)
{
  var crypt = new SHA256Managed();
  var hash = new StringBuilder();
  var crypto = crypt.ComputeHash(Encoding.UTF8.GetBytes(data), 
                                 0, 
                                 Encoding.UTF8.GetByteCount(data));
  foreach (var theByte in crypto)
  {
    hash.Append(theByte.ToStringInvariant("x2"));
  }
  return hash.ToString();
}

V3114 IDisposable object 'crypt' is not disposed before method returns. Extensions.cs 510

Чтобы понять смысл данной диагностики, давайте для начала немного вспомним теорию. С вашего позволения, я возьму информацию из документации к этой диагностике:

"Сборщик мусора автоматически освобождает память, связанную с контролируемым объектом, если он больше не используется, и на него не осталось видимых ссылок. Тем не менее, невозможно предсказать, когда именно произойдёт сборка мусора (если не вызывать её вручную). Кроме того, сборщик мусора не имеет информации о таких неуправляемых ресурсах, как дескрипторы, окна или открытые файлы и потоки. Метод Dispose обычно используется, чтобы освобождать такие неуправляемые ресурсы".

То есть у нас создалась переменная crypt типа SHA256Managed, который реализует интерфейс IDisposable. В итоге, когда мы выйдем из метода, то потенциально захваченные ресурсы не будут освобождены.

Чтобы это предотвратить, советую использовать using. Метод Dispose позовется автоматически при достижении закрывающей фигурной скобки, связанной с инструкцией using. Выглядит это следующим образом:

public static string ToSHA256(this string data)
{
  using (var crypt = new SHA256Managed())
  {
    var hash = new StringBuilder();
    ....
  }
}

А если вы не любите фигурные скобочки, то в C# 8.0 можно написать вот так:

public static string ToSHA256(this string data)
{
  using var crypt = new SHA256Managed();
  var hash = new StringBuilder();
  ....
}

Разница с предыдущим вариантом в том, что метод Dispose позовется при достижении закрывающей фигурной скобки метода. Это конец области, в котором объявляется crypt.

Вещественные числа

public bool ShouldPlot
{
  get
  {
    ....
    if (Time.TimeOfDay.Hours < 10.25) return true;
    ....
  }
}

public struct TimeSpan : IComparable, 
                         IComparable<TimeSpan>, 
                         IEquatable<TimeSpan>, 
                         IFormattable
{
  ....
  public double TotalHours { get; }
  public int Hours { get; }
  ....
}

V3040 The '10.25' literal of the 'double' type is compared to a value of the 'int' type. OpeningBreakoutAlgorithm.cs 426

Выглядит странно, что в условии значение переменной типа int сравнивается с литералом типа double. Выглядит это странно и явно напрашивается какая-то другая переменная. И, действительно, если посмотреть, какие поля с похожим названием есть у TimeOfDay, то мы найдем:

public double TotalHours { get; }

Скорее всего код должен выглядеть вот так:

public bool ShouldPlot
{
  get
  {
    ....
    if (Time.TimeOfDay.TotalHours < 10.25) return true;
    ....
  }
}

Также помните, что нельзя сравнивать на прямое равенство ("==", "!=") числа с плавающей запятой. И не забываем про приведение типов.

Оператор switch

Совет 1

public IEnumerable<TradingDay> GetDaysByType(TradingDayType type,
                                             DateTime start, 
                                             DateTime end)
{
  Func<TradingDay, bool> typeFilter = day =>
  {
    switch (type)        // <=
    {
      case TradingDayType.BusinessDay:
        return day.BusinessDay;
      case TradingDayType.PublicHoliday:
        return day.PublicHoliday;
      case TradingDayType.Weekend:
        return day.Weekend;
      case TradingDayType.OptionExpiration:
        return day.OptionExpirations.Any();
      case TradingDayType.FutureExpiration:
        return day.FutureExpirations.Any();
      case TradingDayType.FutureRoll:
        return day.FutureRolls.Any();
      case TradingDayType.SymbolDelisting:
        return day.SymbolDelistings.Any();
      case TradingDayType.EquityDividends:
        return day.EquityDividends.Any();
    };
    return false;
  };
  return GetTradingDays(start, end).Where(typeFilter);
}

V3002 The switch statement does not cover all values of the 'TradingDayType' enum: EconomicEvent. TradingCalendar.cs 79

Тип переменной type - TradingDayType, и это - enum:

public enum TradingDayType
{
  BusinessDay,
  PublicHoliday,
  Weekend,
  OptionExpiration,
  FutureExpiration,
  FutureRoll,
  SymbolDelisting,
  EquityDividends,
  EconomicEvent
}

Если вы посчитаете, то увидите, что в перечислении имеется 9 элементов, а в switch анализируется только 8. Такая ситуация могла возникнуть из-за расширения кода. Чтобы это предотвратить, я всегда советую явно использовать default:

public IEnumerable<TradingDay> GetDaysByType(TradingDayType type,
                                             DateTime start, 
                                             DateTime end)
{
  Func<TradingDay, bool> typeFilter = day =>
  {
    switch (type)
    {
      ....
      default:
        return false;
    };
  };
  return GetTradingDays(start, end).Where(typeFilter);
}

Как вы могли заметить, оператор return, который был после switch, переехал в default секцию. В данном случае логика программы не изменилась, но я все равно советую писать именно так.

Причина этому – расширяемость кода. В случае оригинала можно спокойно добавить какую-то логику перед return false, не подозревая, что это является default'ом оператора switch. Сейчас же все явно и наглядно.

Однако, если вы считаете, что в вашем случае всегда должна обрабатываться только часть элементов перечисления, то можно кидать исключение:

default:
  throw new CustomExeption("Invalid enumeration element");

Лично я подсел вот на такой синтаксический сахарок C# 8.0:

Func<TradingDay, bool> typeFilter = day =>
{
  return type switch
  {
    TradingDayType.BusinessDay      => day.BusinessDay,
    TradingDayType.PublicHoliday    => day.PublicHoliday,
    TradingDayType.Weekend          => day.Weekend,
    TradingDayType.OptionExpiration => day.OptionExpirations.Any(),
    TradingDayType.FutureExpiration => day.FutureExpirations.Any(),
    TradingDayType.FutureRoll       => day.FutureRolls.Any(),
    TradingDayType.SymbolDelisting  => day.SymbolDelistings.Any(),
    TradingDayType.EquityDividends  => day.EquityDividends.Any(),
    _ => false
  };
};

Совет 2

public string[] GetPropertiesBy(SecuritySeedData type)
{
  switch (type)
  {
    case SecuritySeedData.None:
      return new string[0];

    case SecuritySeedData.OpenInterest:
      return new[] { "OpenInterest" };  // <=

    case SecuritySeedData.OpenInterestTick:
      return new[] { "OpenInterest" };  // <=

    case SecuritySeedData.TradeTick:
      return new[] {"Price", "Volume"};

    ....

    case SecuritySeedData.Fundamentals:
      return new string[0];

    default:
      throw new ArgumentOutOfRangeException(nameof(type), type, null);
  }
}

V3139 Two or more case-branches perform the same actions. SecurityCacheTests.cs 510

В двух разных case возвращается одно и тоже значение. В таком виде это выглядит очень подозрительно. Сразу возникает ощущение, что скопировали, вставили и забыли изменить. Поэтому я рекомендую, если для разных значений должна выполняться одна и та же логика, то объединяйте case вот так:

public string[] GetPropertiesBy(SecuritySeedData type)
{
  switch (type)
  {
    case SecuritySeedData.None:
      return new string[0];

    case SecuritySeedData.OpenInterest:
    case SecuritySeedData.OpenInterestTick:
      return new[] { "OpenInterest" };

    ....
  }
}

Это явно говорит о том, чего мы хотим, плюс убирает лишнюю строчку текста. :)

Оператор if

Пример 1

[TestCaseSource(nameof(DataTypeTestCases))]
public void HandlesAllTypes<T>(....) where T : BaseData, new()
{
  ....
  if (   symbol.SecurityType != SecurityType.Equity
      || resolution != Resolution.Daily
      || resolution != Resolution.Hour)
  {
    actualPricePointsEnqueued++;
    dataPoints.Add(dataPoint);
  }
  ....
}

V3022 Expression 'symbol.SecurityType != SecurityType.Equity || resolution != Resolution.Daily || resolution != Resolution.Hour' is always true. LiveTradingDataFeedTests.cs 1431

Данное условие всегда истинно. Ведь, чтобы условие не выполнилось, переменная resolution должна иметь значение Resolution.Daily и Resolution.Hour одновременно. Возможный исправленный вариант:

[TestCaseSource(nameof(DataTypeTestCases))]
public void HandlesAllTypes<T>(....) where T : BaseData, new()
{
  ....
  if (   symbol.SecurityType != SecurityType.Equity
      || (   resolution != Resolution.Daily 
          && resolution != Resolution.Hour))
  {
    actualPricePointsEnqueued++;
    dataPoints.Add(dataPoint);
  }
  ....
}

Несколько рекомендаций по оператору if. Когда имеется условие, которое полностью состоит из операторов "||", то после написание проверьте, не проверяется ли одна и та же переменная на неравенство чему-то несколько раз подряд.

Похожим образом дело обстоит в условии с оператором "&&". Если переменная несколько раз проверяется на равенство чему-то, то это, скорее всего, логическая ошибка.

Так же, если вы пишете составное условие, и в нем присутствует "&&" и "||", то не смущайтесь ставить скобки. Это может помочь либо увидеть ошибку, либо избежать ее.

Пример 2

public static string SafeSubstring(this string value, 
                                   int startIndex,
                                   int length)
{
  if (string.IsNullOrEmpty(value))
  {
    return value;
  }

  if (startIndex > value.Length - 1)
  {
    return string.Empty;
  }

  if (startIndex < -1)
  {
    startIndex = 0;
  }

  return value.Substring(startIndex, 
                         Math.Min(length, value.Length - startIndex));
}

V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the first argument. StringExtensions.cs 311

Анализатор говорит, что в первый аргумент метода Substring может быть передано значение -1. Это приведет к возникновению исключения типа System.ArgumentOutOfRangeException. Давайте смотреть, почему такое значение может получиться. В этом примере нас не интересуют первые два условия, поэтому они будут опущены в рассуждениях.

Параметр startIndex имеет тип int, следовательно, его значения лежат в диапазоне [-2147483648, 2147483647]. Поэтому, чтобы предотвратить выход за границы массива, разработчик написал следующее условие:

if (startIndex < -1)
{
  startIndex = 0;
}

То есть предполагалось, если пришло отрицательное значение, то мы просто изменим его на 0. Вот только вместо "<=" написали "<", и теперь нижняя граница диапазона переменной startIndex (с точки зрения анализатора) равна -1.

Я предлагаю в подобных ситуациях использовать конструкцию вида:

if (variable < value)
{
  variable = value;
}

Такое сочетание читается намного проще, так как участвует на одно значение меньше. Поэтому предлагаю исправить проблему вот так:

public static string SafeSubstring(....)
{
  ....
  if (startIndex < 0)
  {
    startIndex = 0;
  }

  return value.Substring(startIndex, 
                         Math.Min(length, value.Length - startIndex));
}

Вы можете сказать, что мы могли в начальном примере просто поменять знак в условии:

if (startIndex <= -1)
{
  startIndex = 0;
}

Ошибка при этом также исчезает. Однако логика будет выглядеть вот так:

if (variable <= value - 1)
{
  variable = value;
}

Согласитесь, что это смотрится перегружено.

Пример 3

public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  var futureMarginModel = buyingPowerModel as FutureMarginModel;
  if (buyingPowerModel == null)
  {
    throw new Exception($"Invalid buying power model. " +
                        $"Found: {buyingPowerModel.GetType().Name}. " +
                        $"Expected: {nameof(FutureMarginModel)}");  
  }
  ....
}

V3080 Possible null dereference. Consider inspecting 'buyingPowerModel'. BasicTemplateFuturesAlgorithm.cs 107

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'buyingPowerModel', 'futureMarginModel'. BasicTemplateFuturesAlgorithm.cs 105

Очень любопытный фрагмент. Анализатор выдает сразу два предупреждения. И по факту в них содержится проблема и ее причина. Сначала посмотрим, что будет, если условие выполнится. Так как buyingPowerModel внутри будет строго null, то произойдет разыменование:

$"Found: {buyingPowerModel.GetType().Name}. "

Причина в том, что в условии перепутана переменная, которая сравнивается с null. Вместо buyingPowerModel явно должна быть написана futureMarginModel. Исправленный вариант:

public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  var futureMarginModel = buyingPowerModel as FutureMarginModel;
  if (futureMarginModel == null)
  {
    throw new Exception($"Invalid buying power model. " +
                        $"Found: {buyingPowerModel.GetType().Name}. " +
                        $"Expected: {nameof(FutureMarginModel)}");  
  }
  ....
}

Правда, тут остается проблема с разыменованием buyingPowerModel внутри условия. Ведь futureMarginModel будет null не только когда не является FutureMarginModel, но и когда buyingPowerModel равен null. Поэтому предлагаю вот такой вариант:

public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  var futureMarginModel = buyingPowerModel as FutureMarginModel;
  if (futureMarginModel == null)
  {
    string foundType =    buyingPowerModel?.GetType().Name 
                       ?? "the type was not found because the variable is null";
    throw new Exception($"Invalid buying power model. " +
                        $"Found: {foundType}. " +
                        $"Expected: {nameof(FutureMarginModel)}");   
  }
  ....
}

Лично я в последнее время полюбил писать такие конструкции с помощью is. Так и кода становится меньше, и сложнее совершить ошибку. Данный пример полностью аналогичен примеру выше:

public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  if (!(buyingPowerModel is FutureMarginModel futureMarginModel))
  {
    ....
  }
  ....
}

Тем более, что в C# 9.0 нам добавят возможность писать ключевое слово not:

public override void OnEndOfAlgorithm()
{
  var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
  if (buyingPowerModel is not FutureMarginModel futureMarginModel)
  {
    ....
  }
  ....
}

Пример 4

public static readonly Dictionary<....> 
  FuturesExpiryDictionary = new Dictionary<....>()
{
  ....
  if (twoMonthsPriorToContractMonth.Month == 2)
  {
    lastBusinessDay = FuturesExpiryUtilityFunctions
                      .NthLastBusinessDay(twoMonthsPriorToContractMonth, 1);
  }
  else
  {
    lastBusinessDay = FuturesExpiryUtilityFunctions
                      .NthLastBusinessDay(twoMonthsPriorToContractMonth, 1);
  }
  ....
}

V3004 The 'then' statement is equivalent to the 'else' statement. FuturesExpiryFunctions.cs 1561

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

public static readonly Dictionary<....> 
  FuturesExpiryDictionary = new Dictionary<....>()
{
  ....
  if (twoMonthsPriorToContractMonth.Month == 2)
  {
    lastBusinessDay = FuturesExpiryUtilityFunctions
                      .NthLastBusinessDay(twoMonthsPriorToContractMonth, 2);
  }
  else
  {
    lastBusinessDay = FuturesExpiryUtilityFunctions
                      .NthLastBusinessDay(twoMonthsPriorToContractMonth, 1);
  }
  ....
}

Но это не более, чем предположение. Здесь я бы хотел обратить внимание на то, что ошибка происходит в инициализации контейнера. Размер этой инициализации - почти 2000 строк:

Также фрагменты кода внутри похожи друг на друга, что логично, ведь здесь просто заполняется коллекция. Поэтому будьте особенно осторожны, что-то копируя в больших и похожих участках. Вносите изменения сразу, ибо потом глаза устанут, и вы не увидите проблему.

Пример 5

public AuthenticationToken GetAuthenticationToken(IRestRequest request)
{
  ....
  if (request.Method == Method.GET && request.Parameters.Count > 0)
  {
    var parameters = request.Parameters.Count > 0
                     ? string.Join(....)
                     : string.Empty;
    url = $"{request.Resource}?{parameters}";
  }
}

V3022 Expression 'request.Parameters.Count > 0' is always true. GDAXBrokerage.Utility.cs 63

Условие в тернарном операторе всегда истинно, потому что выше уже была выполнена данная проверка. Теперь это либо избыточная проверка, либо в условии выше перепутаны операторы "&&" и "||".

Чтобы этого избежать, когда вы находитесь в условии, всегда держите в голове при каких значениях вы в него зайдете.

Возможный исправленный вариант:

public AuthenticationToken GetAuthenticationToken(IRestRequest request)
{
  ....
  if (request.Method == Method.GET && request.Parameters.Count > 0)
  {
    var parameters = string.Join(....);
    url = $"{request.Resource}?{parameters}";
  }
}

Пример 6

public bool Setup(SetupHandlerParameters parameters)
{
  ....
  if (job.UserPlan == UserPlan.Free)
  {
    MaxOrders = 10000;
  }
  else
  {
    MaxOrders = int.MaxValue;
    MaximumRuntime += MaximumRuntime;
  }

  MaxOrders = job.Controls.BacktestingMaxOrders; // <=
  ....
}

V3008 The 'MaxOrders' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 244, 240. BacktestingSetupHandler.cs 244

Здесь переменной MaxOrders два раза подряд присваивается значение. То есть логика с условиями является излишней.

Чтобы исправить это, у нас есть 2 варианта. Мы либо убираем присвоения в then-else ветках, либо присвоение после условия. Вероятнее всего, что код покрыт тестами, и программа работает корректно. Поэтому оставим только последние присвоение. Возможный исправленный вариант:

public bool Setup(SetupHandlerParameters parameters)
{
  ....
  if (job.UserPlan != UserPlan.Free)
  {
    MaximumRuntime += MaximumRuntime;
  }

  MaxOrders = job.Controls.BacktestingMaxOrders;
  ....
}

Типичные ошибки для человека

Здесь будут рассмотрены ошибки вида copy-paste, случайно нажатые клавиши и т.д. В общем, самые обычные проблемы несовершенства человека. Мы не машины, поэтому такие ситуации — это нормально.

Общие рекомендации по ним:

если что-то копируете, то вносите изменения в копию сразу же, как только ее вставите;
проводите code review;
используйте специальные инструменты, которые будут искать ошибки за вас.

Ситуация 1

public class FisherTransform : BarIndicator, IIndicatorWarmUpPeriodProvider
{
  private readonly Minimum _medianMin;
  private readonly Maximum _medianMax;
  public override bool IsReady => _medianMax.IsReady && _medianMax.IsReady;
}

V3001 There are identical sub-expressions '_medianMax.IsReady' to the left and to the right of the '&&' operator. FisherTransform.cs 72

В данном примере поле IsReady должно зависеть от двух условий, а по факту зависит от одного. Всему виной опечатка. Скорее всего, вместо _medianMin написали _medianMax. Исправленный вариант:

public class FisherTransform : BarIndicator, IIndicatorWarmUpPeriodProvider
{
  private readonly Minimum _medianMin;
  private readonly Maximum _medianMax;
  public override bool IsReady => _medianMin.IsReady && _medianMax.IsReady;
}

Ситуация 2

public BacktestResultPacket(....) : base(PacketType.BacktestResult)
{
  try
  {
    Progress = Math.Round(progress, 3);
    SessionId = job.SessionId; // <=
    PeriodFinish = endDate;
    PeriodStart = startDate;
    CompileId = job.CompileId;
    Channel = job.Channel;
    BacktestId = job.BacktestId;
    Results = results;
    Name = job.Name;
    UserId = job.UserId;
    ProjectId = job.ProjectId;
    SessionId = job.SessionId; // <=
    TradeableDates = job.TradeableDates;
  }
  catch (Exception err) 
  {
    Log.Error(err);
  }
}

V3008 The 'SessionId' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 182, 172. BacktestResultPacket.cs 182

У класса много полей, которые необходимо проинициализировать – много строк в конструкторе. Все сливается, и одно поле инициализируется несколько раз. В данном случае, здесь может быть лишняя инициализация или же забыли проинициализировать какое-то другое поле.

Если вам интересно, то вы можете посмотреть и другие ошибки, найденные этим диагностическим правилом.

Ситуация 3

private const string jsonWithScore =
            "{" +
            "\"id\":\"e02be50f56a8496b9ba995d19a904ada\"," +
            "\"group-id\":\"a02be50f56a8496b9ba995d19a904ada\"," +
            "\"source-model\":\"mySourceModel-1\"," +
            "\"generated-time\":1520711961.00055," +
            "\"created-time\":1520711961.00055," +
            "\"close-time\":1520711961.00055," +
            "\"symbol\":\"BTCUSD XJ\"," +
            "\"ticker\":\"BTCUSD\"," +
            "\"type\":\"price\"," +
            "\"reference\":9143.53," +
            "\"reference-final\":9243.53," +
            "\"direction\":\"up\"," +
            "\"period\":5.0," +
            "\"magnitude\":0.025," +
            "\"confidence\":null," +
            "\"weight\":null," +
            "\"score-final\":true," +
            "\"score-magnitude\":1.0," +
            "\"score-direction\":1.0," +
            "\"estimated-value\":1113.2484}";

private const string jsonWithExpectedOutputFromMissingCreatedTimeValue =
            "{" +
            "\"id\":\"e02be50f56a8496b9ba995d19a904ada\"," +
            "\"group-id\":\"a02be50f56a8496b9ba995d19a904ada\"," +
            "\"source-model\":\"mySourceModel-1\"," +
            "\"generated-time\":1520711961.00055," +
            "\"created-time\":1520711961.00055," +
            "\"close-time\":1520711961.00055," +
            "\"symbol\":\"BTCUSD XJ\"," +
            "\"ticker\":\"BTCUSD\"," +
            "\"type\":\"price\"," +
            "\"reference\":9143.53," +
            "\"reference-final\":9243.53," +
            "\"direction\":\"up\"," +
            "\"period\":5.0," +
            "\"magnitude\":0.025," +
            "\"confidence\":null," +
            "\"weight\":null," +
            "\"score-final\":true," +
            "\"score-magnitude\":1.0," +
            "\"score-direction\":1.0," +
            "\"estimated-value\":1113.2484}";

V3091 Empirical analysis. It is possible that a typo is present inside the string literal. The 'score' word is suspicious. InsightJsonConverterTests.cs 209

Простите за большой и страшный код. Здесь разные поля имеют одинаковые значения. Это классическая ошибка из семейства copy-paste. Скопировали, задумались, забыли внести изменения – вот и ошибка.

Ситуация 4

private void ScanForEntrance()
{
  var shares = (int)(allowedDollarLoss/expectedCaptureRange);
  ....
  if (ShouldEnterLong)
  {
    MarketTicket = MarketOrder(symbol, shares);
    ....
  }
  else if (ShouldEnterShort)
  {
    MarketTicket = MarketOrder(symbol, - -shares); // <=
    ....
    StopLossTicket = StopMarketOrder(symbol, -shares, stopPrice);
    ....
  }
  ....
}

V3075 The '-' operation is executed 2 or more times in succession. Consider inspecting the '- -shares' expression. OpeningBreakoutAlgorithm.cs 328

Два раза подряд применен унарный оператор "-". Тем самым значение, переданное в метод MarketOrder, останется неизменным. Очень сложно сказать, сколько тут необходимо оставить унарных минусов. Возможно, тут вообще необходим префиксный оператор декремента "--", но была случайно задета клавиша пробел. Вариантов - тьма, поэтому один из возможных исправленных вариантов:

private void ScanForEntrance()
{
  ....
  if (ShouldEnterLong)
  {
    MarketTicket = MarketOrder(symbol, shares);
    ....
  }
  else if (ShouldEnterShort)
  {
    MarketTicket = MarketOrder(symbol, -shares);
    ....
    StopLossTicket = StopMarketOrder(symbol, -shares, stopPrice);
    ....
  }
  ....
}

Ситуация 5

private readonly SubscriptionDataConfig _config;
private readonly DateTime _date;
private readonly bool _isLiveMode;
private readonly BaseData _factory;

public ZipEntryNameSubscriptionDataSourceReader(
    SubscriptionDataConfig config, 
    DateTime date,
    bool isLiveMode)
{
  _config = config;
  _date = date;
  _isLiveMode = isLiveMode;
  _factory = _factory = config.GetBaseDataInstance(); // <=
}

V3005 The '_factory' variable is assigned to itself. ZipEntryNameSubscriptionDataSourceReader.cs 50

Полю _factory два раза присваивается одно и то же значение. В классе всего четыре поля, поэтому, скорее всего, это просто опечатка. Исправленный вариант:

public ZipEntryNameSubscriptionDataSourceReader(....)
{
  _config = config;
  _date = date;
  _isLiveMode = isLiveMode;
  _factory = config.GetBaseDataInstance();
}

Вывод

Существует множество мест, где можно сделать ошибку. Часть мы замечаем и исправляем сразу. Часть исправляется на code review, а часть я рекомендую возложить на специальные инструменты.

Также, если вам понравился подобный формат, то, пожалуйста, напишите об этом. Я сделаю еще подобное. Спасибо!

#CSharp