# Talking about errors in the QuantConnect Lean code

This article discusses errors found using a static analyzer in an open source project\. There are some simple things that can help you avoid them\. For example, the usage of language syntactic constructs starting from C\# 8\.0\. We hope it will be exciting\. Have fun reading\! 

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

[QuantConnect Lean](https://github.com/QuantConnect/Lean/) is an open source algorithmic trading engine designed for easy strategy research, backtesting, and live trading\. Compatible with Windows, Linux, and macOS\. Integrates with prevalent data providers and brokerage companies to quickly deploy algorithmic trading strategies\. 

The check was implemented by using the [PVS\-Studio static analyzer](https://pvs-studio.com/en/pvs-studio/)\. PVS\-Studio is a tool designed to detect errors and potential vulnerabilities in the source code of programs, written in C, C\+\+, C\#, and Java\. 

## Accidents are not accidental\.

```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.com/en/docs/warnings/v3022/) Expression '\_random\.Next\(0, 1\) \=\= 0' is always true\. RandomValueGenerator\.cs 142

The bottom line was that either one or the other value was selected with a 50% probability\. However, in this case, the _Next_ method will always return 0\. 

This happens because the second argument is not included in the range of values\. That is, the value that the method can return will be in the range \[0,1\)\. Let's fix that:

```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;
  }
  ....
}
```

## Passing reference\-type parameters

**Example**

```cpp
/// <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](https://pvs-studio.com/en/docs/warnings/v3061/) Parameter 'array' is always rewritten in method body before being used\. SecurityPortfolioManager\.cs 192

The method accepts the collection and immediately overwrites its value\. Accept that this looks rather suspiciously\. So, let's try to understand what this method must do\.

According to the comment and the method's name, it becomes clear that another array must be copied to the array passed\. However, this will not happen, and the value of the _array_ outside the current method will remain unchanged\.

It happens because the _array_ argument will be passed to the method by value, not by reference\. Thus, when the assign operation is done, the _array_ variable accessible inside the method will store a reference to the new object\. The argument value passed to the method will remain unchanged\. To fix this, the reference type argument must be passed by reference:

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

Since we are certainly creating a new array in the method, the _out _modifier must be used instead of _ref_\. This immediately indicates that the variable inside will be assigned a value\.

By the way, this case enlarges the collection of my colleague, Andrey Karpov, you can learn about it from the article "[Getting started collecting errors in copy functions](https://pvs-studio.com/en/blog/posts/cpp/0723/)"\.

## Unbinding the resources

```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.com/en/docs/warnings/v3114/) IDisposable object 'crypt' is not disposed before method returns\. Extensions\.cs 510

To understand the meaning of this diagnostic, let's first recall a little bit of theory\. If you don't mind, I will take information from the documentation for this diagnostic: 

"The garbage collector automatically unbinds the memory associated with a controlled object if it is no longer in use and there are no visible references to it\. However, we can't say for sure when exactly the garbage collection will occur \(unless you call it manually\)\. In addition, the garbage collector does not have information about unmanaged resources such as handles, windows, or open files and threads\. The _Dispose_ method is usually used to unbind such unmanaged resources"\. 

In other words, we have created a _crypt_ variable of the _SHA256Managed_ type, which implements the _IDisposable_ interface\. As a result, when we exit the method, the potentially captured resources will not be released\. 

To prevent this, I recommend choosing _using_\. The _Dispose_ method is activated automatically upon reaching the closing curly bracket associated with the _using_ instruction\. Let's take a look at this:

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

But if you don't like curly brackets, then in C\# 8\.0 you can write like this:

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

The difference with the previous version is that the _Dispose_ method is activated when the closing curly bracket of the method is reached\. This is the end of the piece where _crypt_ is declared\.

## Real numbers

```cpp
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](https://pvs-studio.com/en/docs/warnings/v3040/) The '10\.25' literal of the 'double' type is compared to a value of the 'int' type\. OpeningBreakoutAlgorithm\.cs 426

It looks strange that in the condition the value of an _int_ variable is compared with a _double\-type_ literal\. This looks odd and another variable is clearly supposed to be here\. And, indeed, if you check out which similarly named fields _TimeOfDay_ has, we will find:

```cpp
public double TotalHours { get; }
```

The code probably should look like this:

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

Also keep in mind that [you mustn't check](https://rsdn.org/article/alg/float.xml) floating\-point numbers for direct equality \("\=\=", "\!\="\)\. And don't forget about [typecasting](https://pvs-studio.com/en/blog/examples/v3041/)\.

## Switch statement

**Tip 1**

```cpp
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](https://pvs-studio.com/en/docs/warnings/v3002/) The switch statement does not cover all values of the 'TradingDayType' enum: EconomicEvent\. TradingCalendar\.cs 79

The type of the variable _type_ is _TradingDayType_, and it is _enum_:

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

If you count, you will see that there are 9 elements in the enumeration, but only 8 are checked in _switch_\. Such situation could happen due to code extension\. To prevent this, I always recommend explicitly using _default_:

```cpp
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);
}
```

As you may have noticed, the _return_ statement that stood after the _switch_ moved to the _default_ section\. In this case, the program's logic has not changed, but I still recommend writing it this way\.

Code extensibility is the reason for it\. In the case of the original, it's possible to safely add some logic before _return false_, without suspecting that this is the _default_ of the _switch_ statement\. Now everything is evident and clear\.

However, if you think that only part of the enumeration elements should always be processed in your case, you can throw an exception:

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

Personally, I got hooked on this syntactic sugar C\# 8\.0:

```cpp
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
  };
};
```

**Tip 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.com/en/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. SecurityCacheTests\.cs 510

In two different _cases_, the same value is returned\. It looks very suspicious in such a form\. It feels like someone copied, pasted and forgot to change the code\. Therefore, here's my advice: if the same logic must be performed for different values, then combine the _case_ that way:

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

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

    ....
  }
}
```

This clearly indicates what we want and removes an extra line as well\. :\)

## If statement

**Example 1**

```cpp
[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](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'symbol\.SecurityType \!\= SecurityType\.Equity \|\| resolution \!\= Resolution\.Daily \|\| resolution \!\= Resolution\.Hour' is always true\. LiveTradingDataFeedTests\.cs 1431

The condition is always true\. After all, in order to have the condition failed, the variable _resolution_ must have the _Resolution\.Daily_ value and _Resolution\.Hour_ at a time\. A possible fixed variant:

```cpp
[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);
  }
  ....
}
```

Some tips for the _if_ statement\. When there is a condition that consists entirely of "\|\|" operators, then after writing, verify whether the same variable is checked for **inequality** to something else several times in a row\. 

The situation is similar in the condition with the "&&" operator\. If a variable is checked for **equality** to something repeatedly, it is most likely to be a logical error\.

Also, if you write a compound condition, and it contains "&&"and "\|\|", do not hesitate to put parentheses\. This can help you either to see an error or to avoid it\.

**Example 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.com/en/docs/warnings/v3057/) The 'Substring' function could receive the '\-1' value while non\-negative value is expected\. Inspect the first argument\. StringExtensions\.cs 311

According to the analyzer the value \-1 can be passed as the first argument of the _Substring_ method\. This will cause an exception of the type _System\.ArgumentOutOfRangeException_\. Let's see why this value can be obtained\. In this example, we take no interest in the first two conditions, so they will be omitted in the reasoning\.

The _startIndex_ parameter has the _int_ type, so its values are in the \[\-2147483648, 2147483647\] range\. Therefore, to prevent bounds violation, the developer wrote the following condition:

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

That is, it was assumed that if a negative value was received, we would simply change it to 0\. But instead of "<\=" they wrote "<", and now the range's lower limit of the _startIndex_ variable \(from the analyzer's point of view\) is \-1\. 

In these situations, I suggest using a construction like this:

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

This combination is much easier to read, since it involves one less value\. So, I suggest you fix the problem like this:

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

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

You can say that we could have just changed the sign in the condition in the initial example:

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

The error also disappears\. However, the logic will look like this:

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

Agree that it looks overloaded\.

**Example 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.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'buyingPowerModel'\. BasicTemplateFuturesAlgorithm\.cs 107

[V3019](https://pvs-studio.com/en/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'buyingPowerModel', 'futureMarginModel'\. BasicTemplateFuturesAlgorithm\.cs 105

A very interesting fragment\. The analyzer issues two warnings at once\. And in fact, they contain the problem and its cause\. Firstly, let's see what happens if the condition is met\. Since _buyingPowerModel_ will be strictly _null_ inside, dereferencing will occur:

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

The reason is that the condition has a wrong variable compared to _null_\. It's obvious that _futureMarginModel_ should be written instead of _buyingPowerModel_\. A fixed version:

```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)}");  
  }
  ....
}
```

However, there is still a problem with dereferencing _buyingPowerModel_ inside the condition\. After all, _futureMarginModel_ will be _null_ not only when it is not _FutureMarginModel_, but also when _buyingPowerModel_ is _null_\. So I suggest this version:

```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)}");   
  }
  ....
}
```

Personally, I have recently got to like writing such constructions using _is_\. So the code becomes shorter and it is more difficult to make a mistake\. This example is completely similar to the example above:

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

Moreover, in C\# 9\.0 we will be able to write the keyword _not_:

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

**Example 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.com/en/docs/warnings/v3004/) The 'then' statement is equivalent to the 'else' statement\. FuturesExpiryFunctions\.cs 1561

The same logic runs under different conditions\. Since one of the arguments is a numeric literal, it is likely that another value may be passed\. For example:

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

But this is nothing more than an assumption\. Here I would like to draw attention to the fact that the error occurs in the container's initialization\. The size of this initialization is almost 2000 lines:

![0782_QuantConnect_Lean/image2.png](https://import.viva64.com/docx/blog/0782_QuantConnect_Lean/image2.png)

Also, the code fragments inside are similar in appearance, which is logical, because here the collection is simply filled in\. Therefore, be very careful when copying something in large and similar sections\. Make changes at once, because then your eyes will get tired and you won't see the problem\.

**Example 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.com/en/docs/warnings/v3022/) Expression 'request\.Parameters\.Count \> 0' is always true\. GDAXBrokerage\.Utility\.cs 63

The condition in the ternary operator is always true, because this check was already performed above\. Now this is either a redundant check, or the "&&" and "\|\|" operators are mixed up in the condition above\. 

To avoid this, when you are in a condition, always keep in mind at what values you will enter it\.

A possible fixed variant:

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

**Example 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.com/en/docs/warnings/v3008/) The 'MaxOrders' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 244, 240\. BacktestingSetupHandler\.cs 244

Here, the _MaxOrders_ variable is assigned a value twice in a row\. In other words, logic with conditions is unnecessary\.

To fix this, we have 2 options\. We either remove the assignments in the then\-else branches, or the assignment after the condition\. Most likely, the code is covered by tests, and the program works correctly\. Therefore, we will leave only the last assignment\. A possible fixed variant:

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

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

## Typical human failings

This section will cover copy\-paste errors, accidentally pressed keys, and so on\. Basically, the most common problems of human imperfection\. We are not robots, so these situations are typical\. 

General recommendations on them: 

* When copying something, make changes to the copy as soon as you paste it;
* review the code;
* use special tools that will look for errors for you\. 

**Case 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.com/en/docs/warnings/v3001/) There are identical sub\-expressions '\_medianMax\.IsReady' to the left and to the right of the '&&' operator\. FisherTransform\.cs 72

In this example, the _IsReady_ field must depend on two conditions, but in fact it depends on one\. It's all the fault of a typo\. Most likely, instead of _\_medianMin_,  _\_medianMax_ was written\. A fixed version:

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

**Case 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.com/en/docs/warnings/v3008/) The 'SessionId' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 182, 172\. BacktestResultPacket\.cs 182

The class has many fields that must be initialized – many lines in the constructor\. Everything is merged, and one field is initialized several times\. In this case, there may be an extra initialization, or they forgot to initialize some other field\.

If you are interested, you can also check out [other errors](https://pvs-studio.com/en/blog/examples/v3008/) found by this diagnostic rule\. 

**Case 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.com/en/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

Sorry for the large and scary code\. Here different fields have the same values\. This is a classic error from the copy\-paste family\. Copied, fell into the muse, forgot to make changes – here's the error\. 

**Case 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.com/en/docs/warnings/v3075/) The '\-' operation is executed 2 or more times in succession\. Consider inspecting the '\- \-shares' expression\. OpeningBreakoutAlgorithm\.cs 328

The unary operator "\-" was used twice in a row\. Thus, the value passed to the _MarketOrder_ method remains unchanged\. It's a tricky question how many unary minuses should be left here\. Perhaps the prefix decrement operator "\-\-" was meant to be here, but the _space bar_ was accidentally pressed\. There are so many variants, so one of the possible corrected options is: 

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

**Case 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.com/en/docs/warnings/v3005/) The '\_factory' variable is assigned to itself\. ZipEntryNameSubscriptionDataSourceReader\.cs 50

The _\_factory_ field is assigned the same value twice\. There are only four fields in the class, so it's probably just a misprint\. A fixed version:

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

## Conclusion

There are many places where you can make a mistake\. We notice and fix some of them immediately\. Some of them are fixed in a code review, but I recommend assigning some of them to special tools\. 

Also, if you like such a format, please write about it\. I'll do more like this\. Thank you\!