6

I have something like the following method.

public Node? GetLastNode(bool createNewIfEmpty = false)
{
    // Return last node if any
    if (Nodes.Count > 0)
        return Nodes[Nodes.Count - 1];

    // Return a new appended node, if requested
    if (createNewIfEmpty)
    {
        Nodes.Add(new Node());
        return Nodes[0];
    }

    // Otherwise, return null
    return null;
}

With nullable reference types on, is there any attribute (or other way) to specify that this method never returns null as long as the createNewIfEmpty parameter is true?

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • You can use `Nodes[^1]` to find the last member of the array instead of `Nodes[Nodes.Count - 1]` – Amirhossein Azhdari Feb 20 '21 at 16:40
  • Still not answering your question, but consider `Nodes.Any()` instead of `Nodes.Count > 0` – Flydog57 Feb 20 '21 at 16:58
  • 1
    @Flydog57: Why? Have you looked at the implementation for `Nodes.Any()` and have a reason to think it's more performant? – Jonathan Wood Feb 20 '21 at 17:00
  • `NotNullIfNotNull` attribute [seems to be a closest for your goal](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/attributes/nullable-analysis#specify-conditional-post-conditions-notnullwhen-maybenullwhen-and-notnullifnotnull), but it doesn't accept a `bool` value – Pavel Anikhouski Feb 20 '21 at 17:13
  • @PavelAnikhouski: Right – Jonathan Wood Feb 20 '21 at 17:19
  • @JonathanWood: take a look at the comments from Eric Lippert on the answer to this: https://stackoverflow.com/questions/59571105/how-can-i-validate-if-listint-is-empty. Yeah, list versus array, but still... – Flydog57 Feb 20 '21 at 19:55
  • @Flydog57: In my example, Nodes os a list and can very quickly give you the number of items via the Count property, so it's really a matter of taste. But I'd also bet $100 my way is a hair faster. – Jonathan Wood Feb 20 '21 at 20:18

2 Answers2

1

I have similar frequent use case and solved by using DoesNotReturnIf.

Problem:

static async Task<Foo?> GetAsync(int id, bool throwIfNotFound)
{
    var result = await FindAsync(id);

    if (throwIfNotFound && result == null)
    {
        throw new ArgumentException("Item not found");
    }

    return result;
}

// Usage
var foo = await GetAsync(0, true);
Console.WriteLine(foo.Name);

Here, warning is raised at foo.Name because the compiler cannot know that foo cannot be nulled if throwIfNotFound is true.

enter image description here

Decorating the parameter with DoesNotReturnIf solves it:

static async Task<Foo?> GetAsync(int id, [DoesNotReturnIf(true)] bool throwIfNotFound)
{
    // ...
}

Now compiler knows that if we set it to true, there won't be a returned value and thus foo won't be null.

enter image description here

I tested with your original question and it's working. We are technically "cheating" since the method DOES return an object but it's enough to trick the compiler:

    if (createNewIfNotFound && result == null)
    {
        // Return instead of throw
        return new();
    }
Luke Vo
  • 17,859
  • 21
  • 105
  • 181
  • Is it only just "cheating", or are you telling the compiler to allow it to optimise away code if it can prove that the code is unreachable? You might be introducing hard to diagnose problems using this approach. – Bouke Apr 07 '23 at 06:23
0

I'm not sure the NotNullIfNotNull can solve it for you, but an alternative would be to split the method in two unless you have a hard requirement to control it from the argument:

public Node? GetLastNode()
{
    // Return last node if any. Otherwise, return null
    return Nodes.Count > 0
        ? Nodes[^1]
        : null;
}

public Node GetOrCreateLastNode()
{
    // Add new node if the list is empty
    if (Nodes.Count == 0)
        Nodes.Add(new Node());

    // Return the last node
    return Nodes[^1];
}
Xerillio
  • 4,855
  • 1
  • 17
  • 28