Sponsored By Aspose - File Format APIs for .NET

Aspose are the market leader of .NET APIs for file business formats – natively work with DOCX, XLSX, PPT, PDF, MSG, MPP, images formats and many more!

Fix your code, don’t disable static analysis

Maybe it is my OCD, maybe it is that I would like to think I try to always write clean code, maybe it is something else entirely. But I always cringe when I see people turn off or disable static analysis in their code.

The reason I cringe is because I have to assume that the authors of the static analysis tools (be it ReSharper or Visual Studio or another product) are more knowledgeable in these areas than I am and they better understand why it is bad to so something.

Today I came across this ‘// ReSharper disable once PossibleMultipleEnumeration’ inside a method and not just once, but twice.

Take a look at the code below.

private ReturnValuesForDateJson[] GetReturnValues(IEnumerable<ReferenceNumberAndReturnTypeRecordModel> recordModels)
{
  // ReSharper disable once PossibleMultipleEnumeration
  var dates = recordModels.First()
  	.GetCalculationResult(CalculationProjectionValueType.UnannualizedRange)
  	.ReturnRecords.Select(x => x.ReturnDate).Distinct();

  return dates.Select(aDate => new ReturnValuesForDateJson
  {
      ReturnDate = new MonthJson(aDate.Year,aDate.Month),
      // ReSharper disable once PossibleMultipleEnumeration
      ReturnValues = GetReturnValueForSeriesIdentifier(aDate, recordModels)
  }).ToArray();

}



Notice how the ReSharper warning for PossibleMultipleEnumerations has been disabled 2 times, this is because the method argument is an IEnumerable. If we change this IEnumerable to either IList or ICollection and the errors go away or we can leave the argument as IEnumerable and get the list by calling .ToList() on the Enumerable.

Now why is it an issue iterating over an enumerable multiple times? Because Enumerable collections are evaluated each time you go over them the underlying results could possibly change. Imagine that you pass in a LINQ statement into the method. The nature of LINQ would allow the results to be different each time, thus possibly causing bugs or errors.

Working code, no more static analysis errors

private ReturnValuesForDateJson[] GetReturnValues(ICollection<ReferenceNumberAndReturnTypeRecordModel> recordModels)
{
  var dates = recordModels.First()
  	.GetCalculationResult(CalculationProjectionValueType.UnannualizedRange)
  	.ReturnRecords.Select(x => x.ReturnDate).Distinct();

  return dates.Select(aDate => new ReturnValuesForDateJson
  {
      ReturnDate = new MonthJson(aDate.Year,aDate.Month),

      ReturnValues = GetReturnValueForSeriesIdentifier(aDate, recordModels)
  }).ToArray();

}



Remember, if the tool is telling you that your code is less than optimal give it a look and try to fix it. Now, there may be legitimate reasons to ignore the warnings, that is cool. But when this is the case do your friends a favor and add a comment regarding the intent so future developers understand the line of thinking.

Till next time,

This entry was posted in Clean Code and tagged . Bookmark the permalink. Follow any comments here with the RSS feed for this post.

Leave a Reply