Bugs in QuantConnect Lean Code

This article discusses bugs in an open source project found using the static analyzer. Here are some simple things that can help you avoid them. For example, using syntactic language constructs since C # 8.0. Hope it will be interesting. Happy reading.



QuantConnect Lean is an open source algorithmic trading engine built for easy strategy research, backtesting, and live trading. Compatible with Windows, Linux and macOS. Integrates with mainstream data providers and brokerage companies to quickly deploy algorithmic trading strategies.



The check was carried out using the PVS-Studio static analyzer . PVS-Studio is a tool for identifying errors and potential vulnerabilities in the source code of programs written in C, C ++, C # and Java on Windows, Linux and macOS.

Accidents are not accidental

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



The point was that either one or another value is selected with 50% probability. However, in this case, the Next method will always return 0.



This is because the range does not include the second argument. That is, the value that the method can return will be in the range [0,1). Let's fix this:

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

/// <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



The method takes a collection and immediately overwrites its value. Agree that this looks rather suspicious. So let's try to understand what this method should do.



From the comment and the name of the method, it becomes clear that some other array should be copied to the passed array. However, this will not happen, and the value of array outside the current method will remain unchanged.



This happens because the array argument will be passed to the method by value, not by reference. Thus, after performing the assignment operation, the reference to the new object will be stored by the variable array , available inside the method. The value of the argument passed to the method will remain unchanged. To fix this, you need to pass the reference type argument by reference:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Since we are certainly writing a new array in the method, we use the out modifier instead of ref . This immediately means that the variable will be assigned a value inside.



By the way, this case adds to the collection that my colleague Andrey Karpov is collecting and about which you can learn from the article " Beginning of Collecting Errors in Copy Functions ".

Freeing up resources

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



To understand the meaning of this diagnostic, let's first recall the theory a little. With your permission, I will take information from the documentation for this diagnostic:



"The garbage collector automatically frees the memory associated with the controlled object when it is no longer used and there are no visible references to it. However, it is impossible to predict exactly when the collection will occur. garbage collection (unless you manually invoke it). Also, the garbage collector has no knowledge of unmanaged resources such as handles, windows, or open files and streams. Dispose is typically used to release such unmanaged resources. "



That is, we have created a crypt variable of type SHA256Managed , which implements the IDisposable interface . As a result, when we exit the method, the potentially acquired resources will not be released.



To prevent this, I recommend using using . The Dispose method will be called automatically when the closing curly brace associated with the using statement is reached . It looks like this:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

And if you don't like curly braces, then in C # 8.0 you can write like this:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

The difference from the previous option is that the Dispose method is called when the method's closing curly brace is reached. This is the end of the region where crypt is declared .

Real numbers

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



It looks strange that in the condition the value of a variable of type int is compared with a literal of type double . It looks strange and some other variable is clearly asking for itself. And, indeed, if we look at what fields with a similar name TimeOfDay has , we will find:

public double TotalHours { get; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Most likely the code should look like this:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Also remember that you cannot compare for direct equality ("==", "! =") Floating point numbers. And don't forget about type casting .

Switch statement

Tip 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 The



type of the variable type is TradingDayType , and this is an enum :

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, and only 8 elements are analyzed in the switch . Such a situation could arise due to the code expansion. To prevent this, I always recommend using default explicitly :

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 after the switch has moved into the default section. In this case, the logic of the program has not changed, but I still advise you to write this way.



The reason for this is the extensibility of the code. In the case of the original, you can safely add some logic before return false , not suspecting that this is the default of the switch statement . Now everything is clear and obvious.



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

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Personally, I got hooked on this C # 8.0 syntactic sugar:

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

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



Two different cases return the same value. In this form, it looks very suspicious. Immediately there is a feeling that you have copied, pasted and forgot to change. Therefore, I recommend that if the same logic should be executed for different values, then combine the case like this:

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, plus it removes the extra line of text. :)

If statement

Example 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



This condition is always true. Indeed, in order for the condition to not be met, the resolution variable must have the value Resolution.Daily and Resolution.Hour at the same time. Possible corrected version:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

A few guidelines for the if statement . When there is a condition that consists entirely of operators "||", then after writing, check if the same variable is checked for inequality for something several times in a row.



The situation is similar with the "&&" operator. If a variable is checked several times for equality to something, then this is most likely a logical error.



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



Example 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



The analyzer says that the value -1 can be passed to the first argument of the Substring method . This will throw an exception of type System.ArgumentOutOfRangeException . Let's see why such a value can turn out. In this example, we are not interested in the first two conditions, so they will be omitted in the reasoning.



Parameter startIndex is of type inttherefore, its values ​​are in the range [-2147483648, 2147483647]. Therefore, in order to prevent overflow of the array boundaries, the developer wrote the following condition:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

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



I suggest using a construction like this in situations like this:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

This combination is much easier to read, since there is one less value involved. Therefore, I propose to fix the problem like this:

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

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

You can say that in the initial example we could simply change the sign in the condition:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

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

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Agree that it looks overwhelmed.



Example 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



A very curious piece. The analyzer generates two warnings at once. And in fact, they contain the problem and its cause. First, let's see what happens if the condition is met. Since buyingPowerModel inside will be strictly null , dereferencing will occur:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

The reason is that a variable is mixed up in the condition, which is compared to null . Instead of buyingPowerModel, the futureMarginModel should be written explicitly . Corrected version:

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 remains a problem with dereferencing buyingPowerModel inside a condition. After all, futureMarginModel will be null not only when it is not a FutureMarginModel , but also when buyingPowerModel is null . Therefore, I suggest this option:

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, lately I have come to love writing such constructs using is . This makes it less code and more difficult to make a mistake. This example is completely similar to the example above:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Moreover, in C # 9.0 we will add the ability to write the not keyword :

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Example 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



Under different conditions, the same logic is executed. Since one of the arguments is a numeric literal, a different value may have to be passed. For example:

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 your attention to the fact that an error occurs in the initialization of the container. The size of this initialization is almost 2000 lines:





Also, the code fragments inside are similar to each other, which is logical, because the collection is simply filled here. Therefore, be especially careful when copying something in large and similar areas. Make changes right away, because then your eyes will get tired and you won't see the problem.



Example 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 The



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



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



Possible corrected version:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Example 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



Here, the MaxOrders variable is assigned a value twice in a row. That is, logic with conditions is redundant.



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 only leave the last assignment. Possible corrected version:

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

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Common mistakes for humans

Errors like copy-paste, accidentally pressed keys, etc. will be considered here. In general, the most common problems of human imperfection. We are not machines, so such situations are normal.



General recommendations for them:

• if you copy something, then make changes to the copy as soon as you paste it;
• conduct a code review;
• use special tools that will look for errors for you.

Situation 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



In this example, the IsReady field must depend on two conditions, but in fact depends on one. It's all the fault of a typo. Most likely, they wrote _medianMax instead of _medianMin . Corrected version:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Situation 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



The class has many fields that need to be initialized - many lines in the constructor. Everything is merged and one field is initialized multiple times. In this case, there may be unnecessary initialization, or they forgot to initialize some other field.



If you are interested, you can look at other errors found by this diagnostic rule.



Situation 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



Sorry for the big and scary code. Different fields have the same values ​​here. This is a classic mistake from the copy-paste family. Copied, thoughtful, forgot to make changes - that's the mistake.



Situation 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 Unary



"-" operator applied twice in a row. Thus, the value passed to the MarketOrder method will remain unchanged. It is very difficult to say how many unary cons should be left here. Perhaps the prefix decrement operator "-" is generally needed here, but the space key was accidentally hit . The options are dark, so one of the possible corrected options:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Situation 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



Paul _factory twice is assigned the same value. There are only four fields in the class, so this is most likely just a typo. Corrected version:

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

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

Conclusion

There are many places where you can make mistakes. Some we notice and correct immediately. Part of it is corrected for code review, and I recommend assigning part to special tools.



Also, if you liked this format, then please write about it. I will do another similar thing. Thank you!

If you want to share this article with an English-speaking audience, please use the translation link: Nikolay Mironov. Talking About Errors in the QuantConnect Lean Code .