-1

I am trying to optimize the code using SonarQube

List<string> itemList = serviceObject.GetItems();

I tried to validate the list with the below code

if(itemList != null && itemList.Any()
{
     //Some operations
}

when above code executed I am getting Sonarqube error remove this expression which always evaluates to "true" So I refactor the code as

if(itemList == null || !itemList.Any())
    return;
//Some Operations

when above code executed I am getting Sonarqube error remove this expression which always evaluates to "false"

Could anyone let me know what is wrong here?

Manikandan Sekar
  • 73
  • 2
  • 16

4 Answers4

2

You can shorten this to

if (itemList?.Count >0)
{
...
}

or

if (itemList?.Any() ==true)
{
...
}

?. is one of the Null conditional operators (the other is ?[]), the Elvis operator for short, that allows you to access potentially null variables without throwing. The result of the entire expression after the Elvis operator is nullable and returns null if the variable is null.

This means that itemList?.Count returns an Nullable<int>, and itemList?.Any() a Nullable<bool>. Nullable<T> defines relational operators between itself and its base type T but can't be used as T without an explicit cast. That's why (itemList?.Any() ==true) is needed.

If you use Nullable Reference Types though, itemList can't be null, so a simple comparison would be enough:

if (itemList.Count >0)
{
...
}

If you enable Nullable Reference Types by setting #nullable enable in a source file or <Nullable>enable</Nullable> in the csproj file, the compiler ensures that all variables, fields and methods that return reference types are not null, forcing you to either fix the problem or explicitly specify that a variable is nullable.

With NRTs enabled this line :

List<string> itemList = serviceObject.GetItems();

Would only compile without warnings if GetItems never returned a null. If the compiler doubts this, it would issue a warning advising you to either fix the problem or explicitly declare itemList as nullable with :

List<string>? itemList = serviceObject.GetItems();
Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
  • I think of the same, but not implemented as I thought it handles True case only and may results in SonarQube code smell. To my surprise it passed SonarQube. Lesson learned instead of thinking will it works of not, better try it and get conclude. Thanks a lot – Manikandan Sekar Mar 05 '21 at 17:00
1

I believe it is due to null Comparision.

itemList != null

There is a high chance that serviceObject.GetItems(); is guaranteed to not return null by having a [NotNull] Annotation. Hence the null check is redundant.

How can I show that a method will never return null (Design by contract) in C#

Code Name Jack
  • 2,856
  • 24
  • 40
  • The `NotNull` attribute in C# has little to do with Code Contracts, which are more-or-less abandoned. It's used with NRTs to [help the compiler](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/attributes/nullable-analysis) when it can't determine the nullability of a result or value. In this case, with NRTs, `List` means the variable can't be null. If `GetItems` returned `List?` the compiler would generate a warning – Panagiotis Kanavos Mar 05 '21 at 07:29
  • NotNull is a resharper thing, not a C# thing (any more), right? – Caius Jard Mar 05 '21 at 07:30
  • 1
    @PanagiotisKanavos Yes you are right about the nullable reference type and the explanation I gave is misleading. – Code Name Jack Mar 05 '21 at 08:12
-1

if( itemList !=null ) {//Do something...}

Alex Dsouza
  • 1
  • 3
-2
List<string> itemList = new List<string>();
itemList = serviceObject.GetItems();
if(itemList!=null && itemList.Count() > 0 )
{
     //Some operations
}

Count is enough to validate a list

List will always not be null but will have no elements. but you have to instatiate it so it wont throw an exception.

JKC
  • 79
  • 6
  • 4
    You cannot invoke any methods (like `Count()` if the list is not instantiated, i.e. if it's `null`). Hence to use `Count()` or `Any()` you must first ensure that the list is not null. – Jakob Busk Sørensen Mar 05 '21 at 07:09
  • @JKC what Jakob said is right. we cant expect the service not return null. Its better to check for null – Manikandan Sekar Mar 05 '21 at 07:12
  • The new List will always not be null if instantiated so Count works just fine if instantiated, it will also be good for code reusability – JKC Mar 05 '21 at 07:15
  • 2
    Careful with Count(); it can be a performance disaster in some cases; see https://stackoverflow.com/questions/305092/which-method-performs-better-any-vs-count-0 – Caius Jard Mar 05 '21 at 07:16
  • @JKC creating a new list only to discard it is wasteful. You can't call `itemList[0]` if the list has no items. What you wrote will throw a confusing `IndexOutOfRangeException` if `itemList` was null – Panagiotis Kanavos Mar 05 '21 at 07:18
  • 1
    The point Jakob is making is "how can you be absolutely sure that `serviceObject.GetItems()` did not return null?" - if it does, your code explodes. Your edit doesn't address this in any way, it's just making an incorrect answer worse – Caius Jard Mar 05 '21 at 07:26
  • @CaiusJard if serviceObject.GetItems() returns a List without a value it will never be a null https://dotnetfiddle.net/75Sa36 – JKC Mar 05 '21 at 07:28
  • @JKC but it can return a `null`. In which case your code would throw an `IndexOutOfRangeException`. The question is why the check evaluates to true, and how to simplify the expression. – Panagiotis Kanavos Mar 05 '21 at 07:31
  • 1
    @JKC that is not the point I'm making at all. Here are the ways your code explodes: https://dotnetfiddle.net/JX5NzG https://dotnetfiddle.net/prqKJq https://dotnetfiddle.net/ydpvCo , and that's even after fixing the error that your code won't compile because Count is a property of list, not a method (your fiddle didn't import LINQ). There's no point making a new list only to overwrite it with return value of GetItems, and even if you did `itemList = service.GetItems()?itemList;` it would *still* blow up because a new list doesn't have an item at [0] – Caius Jard Mar 05 '21 at 07:44