# Об ошибках в коде QuantConnect Lean
В данной статье рассматриваются ошибки в проекте с открытым исходным кодом, найденные с помощью статического анализатора\. Говорится о некоторых простых вещах, которые могут помочь избежать их появления\. Например, используя синтаксические конструкции языка начиная с C\# 8\.0\. Надеюсь, что будет интересно\. Приятного прочтения\.
[QuantConnect Lean](https://github.com/QuantConnect/Lean/) \- это алгоритмический торговый движок с открытым исходным кодом, созданный для легкого исследования стратегий, бэктестинга и живой торговли\. Совместим с Windows, Linux и macOS\. Интегрируется с распространенными поставщиками данных и брокерскими компаниями для быстрого развертывания алгоритмических торговых стратегий\.
Проверка была осуществлена с помощью статического анализатора [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\. PVS\-Studio \- это инструмент для выявления ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках С, C\+\+, C\# и Java на Windows, Linux и macOS\.
## Случайности не случайны
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression '\_random\.Next\(0, 1\) \=\= 0' is always true\. RandomValueGenerator\.cs 142
Смысл был в том, что выбирается либо одно, либо другое значение с 50% вероятностью\. Однако в этом случае метод _Next_ всегда будет возвращать 0\.
Это происходит потому, что в диапазон значений не включается второй аргумент\. То есть значение, которое метод может вернуть, будет находиться в диапазоне \[0,1\)\. Исправим это:
```cpp
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;
}
....
}
```
## Передача параметров ссылочного типа
**Пример**
```cpp
///
/// Copy contents of the portfolio collection to a new destination.
///
///
/// IDictionary implementation calling the underlying Securities collection
///
/// Destination array
/// Position in array to start copying
public void CopyTo(KeyValuePair[] array, int index)
{
array = new KeyValuePair[Securities.Count];
var i = 0;
foreach (var asset in Securities)
{
if (i >= index)
{
array[i] = new KeyValuePair(asset.Key,
asset.Value.Holdings);
}
i++;
}
}
```
[V3061](https://pvs-studio.ru/ru/docs/warnings/v3061/) Parameter 'array' is always rewritten in method body before being used\. SecurityPortfolioManager\.cs 192
Метод принимает коллекцию и сразу же перезаписывает ее значение\. Согласитесь, что это выглядит достаточно подозрительно\. Поэтому давайте постараемся понять, что должен делать этот метод\.
По комментарию и названию метода становится понятно, что в переданный массив должен скопироваться какой\-то другой\. Однако этого не произойдет, и значение у _array_ вне текущего метода останется неизмененным\.
Это происходит из\-за того, что аргумент _array_ будет передан в метод по значению, а не по ссылке\. Таким образом, после выполнения операции присвоения ссылку на новый объект будет хранить переменная _array_, доступная внутри метода\. Значение аргумента, переданного в метод, останется неизменным\. Чтобы это исправить, необходимо передать аргумент ссылочного типа по ссылке:
```cpp
public void CopyTo(out KeyValuePair[] array, // <=
int index)
{
array = new KeyValuePair[Securities.Count];
....
}
```
Так как мы, безусловно, записываем в методе новый массив, то используем модификатор _out_ вместо _ref_\. Это сразу говорит о том, что переменной внутри будет присвоено значение\.
Кстати, этот случай пополняет коллекцию, которую собирает мой коллега Андрей Карпов и про которую можно узнать из статьи "[Начало коллекционирования ошибок в функциях копирования](https://pvs-studio.ru/ru/blog/posts/cpp/0723/)"\.
## Освобождаем ресурсы
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3114/) IDisposable object 'crypt' is not disposed before method returns\. Extensions\.cs 510
Чтобы понять смысл данной диагностики, давайте для начала немного вспомним теорию\. С вашего позволения, я возьму информацию из документации к этой диагностике:
"Сборщик мусора автоматически освобождает память, связанную с контролируемым объектом, если он больше не используется, и на него не осталось видимых ссылок\. Тем не менее, невозможно предсказать, когда именно произойдёт сборка мусора \(если не вызывать её вручную\)\. Кроме того, сборщик мусора не имеет информации о таких неуправляемых ресурсах, как дескрипторы, окна или открытые файлы и потоки\. Метод _Dispose_ обычно используется, чтобы освобождать такие неуправляемые ресурсы"\.
То есть у нас создалась переменная _crypt_ типа _SHA256Managed_, который реализует интерфейс _IDisposable_\. В итоге, когда мы выйдем из метода, то потенциально захваченные ресурсы не будут освобождены\.
Чтобы это предотвратить, советую использовать _using_\. Метод _Dispose_ позовется автоматически при достижении закрывающей фигурной скобки, связанной с инструкцией _using_\. Выглядит это следующим образом:
```cpp
public static string ToSHA256(this string data)
{
using (var crypt = new SHA256Managed())
{
var hash = new StringBuilder();
....
}
}
```
А если вы не любите фигурные скобочки, то в C\# 8\.0 можно написать вот так:
```cpp
public static string ToSHA256(this string data)
{
using var crypt = new SHA256Managed();
var hash = new StringBuilder();
....
}
```
Разница с предыдущим вариантом в том, что метод _Dispose_ позовется при достижении закрывающей фигурной скобки метода\. Это конец области, в котором объявляется _crypt_\.
## Вещественные числа
```cpp
public bool ShouldPlot
{
get
{
....
if (Time.TimeOfDay.Hours < 10.25) return true;
....
}
}
public struct TimeSpan : IComparable,
IComparable,
IEquatable,
IFormattable
{
....
public double TotalHours { get; }
public int Hours { get; }
....
}
```
[V3040](https://pvs-studio.ru/ru/docs/warnings/v3040/) The '10\.25' literal of the 'double' type is compared to a value of the 'int' type\. OpeningBreakoutAlgorithm\.cs 426
Выглядит странно, что в условии значение переменной типа _int_ сравнивается с литералом типа _double_\. Выглядит это странно и явно напрашивается какая\-то другая переменная\. И, действительно, если посмотреть, какие поля с похожим названием есть у _TimeOfDay_, то мы найдем:
```cpp
public double TotalHours { get; }
```
Скорее всего код должен выглядеть вот так:
```cpp
public bool ShouldPlot
{
get
{
....
if (Time.TimeOfDay.TotalHours < 10.25) return true;
....
}
}
```
Также помните, что [нельзя сравнивать на прямое равенство](https://rsdn.org/article/alg/float.xml) \("\=\=", "\!\="\) числа с плавающей запятой\. И не забываем про [приведение типов](https://pvs-studio.ru/ru/blog/examples/v3041/)\.
## Оператор switch
**Совет 1**
```cpp
public IEnumerable GetDaysByType(TradingDayType type,
DateTime start,
DateTime end)
{
Func 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](https://pvs-studio.ru/ru/docs/warnings/v3002/) The switch statement does not cover all values of the 'TradingDayType' enum: EconomicEvent\. TradingCalendar\.cs 79
Тип переменной _type_ \- _TradingDayType_, и это \- _enum_:
```cpp
public enum TradingDayType
{
BusinessDay,
PublicHoliday,
Weekend,
OptionExpiration,
FutureExpiration,
FutureRoll,
SymbolDelisting,
EquityDividends,
EconomicEvent
}
```
Если вы посчитаете, то увидите, что в перечислении имеется 9 элементов, а в _switch_ анализируется только 8\. Такая ситуация могла возникнуть из\-за расширения кода\. Чтобы это предотвратить, я всегда советую явно использовать _default_:
```cpp
public IEnumerable GetDaysByType(TradingDayType type,
DateTime start,
DateTime end)
{
Func typeFilter = day =>
{
switch (type)
{
....
default:
return false;
};
};
return GetTradingDays(start, end).Where(typeFilter);
}
```
Как вы могли заметить, оператор _return_, который был после _switch_, переехал в _default_ секцию\. В данном случае логика программы не изменилась, но я все равно советую писать именно так\.
Причина этому – расширяемость кода\. В случае оригинала можно спокойно добавить какую\-то логику перед _return false_, не подозревая, что это является _default'ом_ оператора _switch_\. Сейчас же все явно и наглядно\.
Однако, если вы считаете, что в вашем случае всегда должна обрабатываться только часть элементов перечисления, то можно кидать исключение:
```cpp
default:
throw new CustomExeption("Invalid enumeration element");
```
Лично я подсел вот на такой синтаксический сахарок C\# 8\.0:
```cpp
Func 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**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. SecurityCacheTests\.cs 510
В двух разных _case_ возвращается одно и тоже значение\. В таком виде это выглядит очень подозрительно\. Сразу возникает ощущение, что скопировали, вставили и забыли изменить\. Поэтому я рекомендую, если для разных значений должна выполняться одна и та же логика, то объединяйте _case_ вот так:
```cpp
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**
```cpp
[TestCaseSource(nameof(DataTypeTestCases))]
public void HandlesAllTypes(....) where T : BaseData, new()
{
....
if ( symbol.SecurityType != SecurityType.Equity
|| resolution != Resolution.Daily
|| resolution != Resolution.Hour)
{
actualPricePointsEnqueued++;
dataPoints.Add(dataPoint);
}
....
}
```
[V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'symbol\.SecurityType \!\= SecurityType\.Equity \|\| resolution \!\= Resolution\.Daily \|\| resolution \!\= Resolution\.Hour' is always true\. LiveTradingDataFeedTests\.cs 1431
Данное условие всегда истинно\. Ведь, чтобы условие не выполнилось, переменная _resolution_ должна иметь значение _Resolution\.Daily_ и _Resolution\.Hour_ одновременно\. Возможный исправленный вариант:
```cpp
[TestCaseSource(nameof(DataTypeTestCases))]
public void HandlesAllTypes(....) where T : BaseData, new()
{
....
if ( symbol.SecurityType != SecurityType.Equity
|| ( resolution != Resolution.Daily
&& resolution != Resolution.Hour))
{
actualPricePointsEnqueued++;
dataPoints.Add(dataPoint);
}
....
}
```
Несколько рекомендаций по оператору _if_\. Когда имеется условие, которое полностью состоит из операторов "\|\|", то после написание проверьте, не проверяется ли одна и та же переменная на **неравенство** чему\-то несколько раз подряд\.
Похожим образом дело обстоит в условии с оператором "&&"\. Если переменная несколько раз проверяется на **равенство** чему\-то, то это, скорее всего, логическая ошибка\.
Так же, если вы пишете составное условие, и в нем присутствует "&&" и "\|\|", то не смущайтесь ставить скобки\. Это может помочь либо увидеть ошибку, либо избежать ее\.
**Пример 2**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/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\]\. Поэтому, чтобы предотвратить выход за границы массива, разработчик написал следующее условие:
```cpp
if (startIndex < -1)
{
startIndex = 0;
}
```
То есть предполагалось, если пришло отрицательное значение, то мы просто изменим его на 0\. Вот только вместо "<\=" написали "<", и теперь нижняя граница диапазона переменной _startIndex_ \(с точки зрения анализатора\) равна \-1\.
Я предлагаю в подобных ситуациях использовать конструкцию вида:
```cpp
if (variable < value)
{
variable = value;
}
```
Такое сочетание читается намного проще, так как участвует на одно значение меньше\. Поэтому предлагаю исправить проблему вот так:
```cpp
public static string SafeSubstring(....)
{
....
if (startIndex < 0)
{
startIndex = 0;
}
return value.Substring(startIndex,
Math.Min(length, value.Length - startIndex));
}
```
Вы можете сказать, что мы могли в начальном примере просто поменять знак в условии:
```cpp
if (startIndex <= -1)
{
startIndex = 0;
}
```
Ошибка при этом также исчезает\. Однако логика будет выглядеть вот так:
```cpp
if (variable <= value - 1)
{
variable = value;
}
```
Согласитесь, что это смотрится перегружено\.
**Пример 3**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'buyingPowerModel'\. BasicTemplateFuturesAlgorithm\.cs 107
[V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'buyingPowerModel', 'futureMarginModel'\. BasicTemplateFuturesAlgorithm\.cs 105
Очень любопытный фрагмент\. Анализатор выдает сразу два предупреждения\. И по факту в них содержится проблема и ее причина\. Сначала посмотрим, что будет, если условие выполнится\. Так как _buyingPowerModel_ внутри будет строго _null_, то произойдет разыменование:
```cpp
$"Found: {buyingPowerModel.GetType().Name}. "
```
Причина в том, что в условии перепутана переменная, которая сравнивается с _null_\. Вместо _buyingPowerModel_ явно должна быть написана _futureMarginModel_\. Исправленный вариант:
```cpp
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_\. Поэтому предлагаю вот такой вариант:
```cpp
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_\. Так и кода становится меньше, и сложнее совершить ошибку\. Данный пример полностью аналогичен примеру выше:
```cpp
public override void OnEndOfAlgorithm()
{
var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
if (!(buyingPowerModel is FutureMarginModel futureMarginModel))
{
....
}
....
}
```
Тем более, что в C\# 9\.0 нам добавят возможность писать ключевое слово _not_:
```cpp
public override void OnEndOfAlgorithm()
{
var buyingPowerModel = Securities[_contractSymbol].BuyingPowerModel;
if (buyingPowerModel is not FutureMarginModel futureMarginModel)
{
....
}
....
}
```
**Пример 4**
```cpp
public static readonly Dictionary<....>
FuturesExpiryDictionary = new Dictionary<....>()
{
....
if (twoMonthsPriorToContractMonth.Month == 2)
{
lastBusinessDay = FuturesExpiryUtilityFunctions
.NthLastBusinessDay(twoMonthsPriorToContractMonth, 1);
}
else
{
lastBusinessDay = FuturesExpiryUtilityFunctions
.NthLastBusinessDay(twoMonthsPriorToContractMonth, 1);
}
....
}
```
[V3004](https://pvs-studio.ru/ru/docs/warnings/v3004/) The 'then' statement is equivalent to the 'else' statement\. FuturesExpiryFunctions\.cs 1561
При разных условиях выполняется одна и та же логика\. Так как один из аргументов является числовым литералом, возможно, должно быть передано другое значение\. Например:
```cpp
public static readonly Dictionary<....>
FuturesExpiryDictionary = new Dictionary<....>()
{
....
if (twoMonthsPriorToContractMonth.Month == 2)
{
lastBusinessDay = FuturesExpiryUtilityFunctions
.NthLastBusinessDay(twoMonthsPriorToContractMonth, 2);
}
else
{
lastBusinessDay = FuturesExpiryUtilityFunctions
.NthLastBusinessDay(twoMonthsPriorToContractMonth, 1);
}
....
}
```
Но это не более, чем предположение\. Здесь я бы хотел обратить внимание на то, что ошибка происходит в инициализации контейнера\. Размер этой инициализации \- почти 2000 строк:
Также фрагменты кода внутри похожи друг на друга, что логично, ведь здесь просто заполняется коллекция\. Поэтому будьте особенно осторожны, что\-то копируя в больших и похожих участках\. Вносите изменения сразу, ибо потом глаза устанут, и вы не увидите проблему\.
**Пример 5**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'request\.Parameters\.Count \> 0' is always true\. GDAXBrokerage\.Utility\.cs 63
Условие в тернарном операторе всегда истинно, потому что выше уже была выполнена данная проверка\. Теперь это либо избыточная проверка, либо в условии выше перепутаны операторы "&&" и "\|\|"\.
Чтобы этого избежать, когда вы находитесь в условии, всегда держите в голове при каких значениях вы в него зайдете\.
Возможный исправленный вариант:
```cpp
public AuthenticationToken GetAuthenticationToken(IRestRequest request)
{
....
if (request.Method == Method.GET && request.Parameters.Count > 0)
{
var parameters = string.Join(....);
url = $"{request.Resource}?{parameters}";
}
}
```
**Пример 6**
```cpp
public bool Setup(SetupHandlerParameters parameters)
{
....
if (job.UserPlan == UserPlan.Free)
{
MaxOrders = 10000;
}
else
{
MaxOrders = int.MaxValue;
MaximumRuntime += MaximumRuntime;
}
MaxOrders = job.Controls.BacktestingMaxOrders; // <=
....
}
```
[V3008](https://pvs-studio.ru/ru/docs/warnings/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 ветках, либо присвоение после условия\. Вероятнее всего, что код покрыт тестами, и программа работает корректно\. Поэтому оставим только последние присвоение\. Возможный исправленный вариант:
```cpp
public bool Setup(SetupHandlerParameters parameters)
{
....
if (job.UserPlan != UserPlan.Free)
{
MaximumRuntime += MaximumRuntime;
}
MaxOrders = job.Controls.BacktestingMaxOrders;
....
}
```
## Типичные ошибки для человека
Здесь будут рассмотрены ошибки вида copy\-paste, случайно нажатые клавиши и т\.д\. В общем, самые обычные проблемы несовершенства человека\. Мы не машины, поэтому такие ситуации — это нормально\.
Общие рекомендации по ним:
* если что\-то копируете, то вносите изменения в копию сразу же, как только ее вставите;
* проводите code review;
* используйте специальные инструменты, которые будут искать ошибки за вас\.
**Ситуация 1**
```cpp
public class FisherTransform : BarIndicator, IIndicatorWarmUpPeriodProvider
{
private readonly Minimum _medianMin;
private readonly Maximum _medianMax;
public override bool IsReady => _medianMax.IsReady && _medianMax.IsReady;
}
```
[V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions '\_medianMax\.IsReady' to the left and to the right of the '&&' operator\. FisherTransform\.cs 72
В данном примере поле _IsReady_ должно зависеть от двух условий, а по факту зависит от одного\. Всему виной опечатка\. Скорее всего, вместо _\_medianMin_ написали _\_medianMax_\. Исправленный вариант:
```cpp
public class FisherTransform : BarIndicator, IIndicatorWarmUpPeriodProvider
{
private readonly Minimum _medianMin;
private readonly Maximum _medianMax;
public override bool IsReady => _medianMin.IsReady && _medianMax.IsReady;
}
```
**Ситуация 2**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'SessionId' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 182, 172\. BacktestResultPacket\.cs 182
У класса много полей, которые необходимо проинициализировать – много строк в конструкторе\. Все сливается, и одно поле инициализируется несколько раз\. В данном случае, здесь может быть лишняя инициализация или же забыли проинициализировать какое\-то другое поле\.
Если вам интересно, то вы можете посмотреть и [другие ошибки](https://pvs-studio.ru/ru/blog/examples/v3008/), найденные этим диагностическим правилом\.
**Ситуация 3**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/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**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3075/) The '\-' operation is executed 2 or more times in succession\. Consider inspecting the '\- \-shares' expression\. OpeningBreakoutAlgorithm\.cs 328
Два раза подряд применен унарный оператор "\-"\. Тем самым значение, переданное в метод _MarketOrder_, останется неизменным\. Очень сложно сказать, сколько тут необходимо оставить унарных минусов\. Возможно, тут вообще необходим префиксный оператор декремента "\-\-", но была случайно задета клавиша _пробел_\. Вариантов \- тьма, поэтому один из возможных исправленных вариантов:
```cpp
private void ScanForEntrance()
{
....
if (ShouldEnterLong)
{
MarketTicket = MarketOrder(symbol, shares);
....
}
else if (ShouldEnterShort)
{
MarketTicket = MarketOrder(symbol, -shares);
....
StopLossTicket = StopMarketOrder(symbol, -shares, stopPrice);
....
}
....
}
```
**Ситуация 5**
```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3005/) The '\_factory' variable is assigned to itself\. ZipEntryNameSubscriptionDataSourceReader\.cs 50
Полю _\_factory_ два раза присваивается одно и то же значение\. В классе всего четыре поля, поэтому, скорее всего, это просто опечатка\. Исправленный вариант:
```cpp
public ZipEntryNameSubscriptionDataSourceReader(....)
{
_config = config;
_date = date;
_isLiveMode = isLiveMode;
_factory = config.GetBaseDataInstance();
}
```
## Вывод
Существует множество мест, где можно сделать ошибку\. Часть мы замечаем и исправляем сразу\. Часть исправляется на code review, а часть я рекомендую возложить на специальные инструменты\.
Также, если вам понравился подобный формат, то, пожалуйста, напишите об этом\. Я сделаю еще подобное\. Спасибо\!