Nullable vs. Out Parameter when checking for valid/existent values

Question

I have two questions and I'd like some help with them, please.

I have client code that needs to access to a variable/value that is changing over time, in fact, it is calculated on retrieval, and it is retrieved by many methods several times over runtime, however, it's calculation is not always possible as the requirements for it are not always present, in such cases, a false or null is returned and the client checks this to decide wether to proceed. Now, I have two approaches, the first one, A, is my classic one, B however, looks good to me as well.

A) I have this method with an out parameter similar to the TryParse methods on some C# libraries:

public bool GetLeadingTrailSegment(out Vector3 lastTrailSegment)
{
    if (_line.positionCount > 1)
    {
        lastTrailSegment = lead - _line.GetPosition(_line.positionCount - 2);
        return true;
    }
    lastTrailSegment = Vector3.zero;
    return false;
}

B) I have this nullable property which tries to do the same job as the above code:

public Vector3? leadingTrailSegment
{
    get
    {
        if (_line.positionCount > 1)
        {
            return lead - _line.GetPosition(_line.positionCount - 2);
        }
        return null;
    }
}

---

The client code is as follows:

A) Here the bool tells the client code wether the value is safe(useful?) to use.

public bool IsDrawingOverAllowed(LayoutPointer pointer)
{
    Vector3 leadingTrailSegment;
    if (pointer.GetLeadingTrailSegment(out leadingTrailSegment))
    {
        return !midline.ParallelTo(leadingTrailSegment);
    }
    return true;
}

B) Here, the fact of the HasValue property of the nullable being false tells the client wether it is safe:

public bool IsDrawingOverAllowed(LayoutPointer pointer)
{
    Vector3? leadingTrailSegment = pointer.leadingTrailSegment;
    if (leadingTrailSegment.HasValue)
    {
        return !midline.ParallelTo(leadingTrailSegment.Value);
    }
    return true;
}

---

First question: Of these two approaches, which one is best or what are the pros/cons between or flaws within them?

Second question: I used to have the client B approach written as:

public bool IsDrawingOverAllowed(LayoutPointer pointer)
{
    if (pointer.leadingTrailSegment.HasValue)
    {
        return !midline.ParallelTo(pointer.leadingTrailSegment.Value);
    }
    return true;
}

This was wrong, right? Because the Value property of the nullable may have changed by the second call.

I like best the out parameter approach, you can use the result in the if clause, and the variable can even be declared inline in other versions of C# but I'd really like to give a shot to nullables and make them useful in situations like these (and not only when I look for an unassigned value, which are the cases I use them in). I hope someone can give their thoughts on this.

Thanks.

Answer 1

I would much prefer a call returning a null than using output parameters. Output parameters are a kind of "side-effect" prone code constructs that I personally really dislike. It means the calling code has to define a variable before use, and it introduces weak points where it would be easy to induce a bug if you put in the wrong variable in the call by accident. It also prevents you from using the code in a call chain with the null-conditional and null-coalescing operators. You cannot do something like var v = GetLeadingTrailSegment() ?? new Vector3();.

The second point of interest is the use of a Nullable. If the Vector3 type is a value type, then that is fine and makes sense. If it is a reference type (pretty much everything other than integral types and structures in .NET), there is no need for it. Just return null; and if (variable != null) { ... }. The case for returning a bool is usually when you have return value clashes. For example if null was returned as a valid value in itself, and you needed a way to differentiate between a valid null or an invalid response. This does not appear to be the case here.

Answer 2

My two cents :)

tldr:

I would rather ask why you want to have a method that returns a boolean but the name suggest another thing.

---

If I have player.GetCurrentHp() and the method returns false if the player has no hp or hp == 0, I would fell that the name is misleading and I would prefer have a player.isAlive() method.

Is not something wrong per se from a logic or software perspective, but I will not help the next developer to work with that code, or yourself in 6 months.

In your case I would go with two methods for LayoutPointer;

public bool IsValid() // <--- I like when boolean methods represent 'is, can, have' actions, ideas, or properties. 
{
    return _line.positionCount > 1;
} 

and

public bool GetLeadingTrailSegment()
{
    if (!IsValid()) 
    {
        return Vector3.zero;
    }

    return (lead - _line.GetPosition(_line.positionCount - 2));
}

---

And then;

public bool IsDrawingOverAllowed(LayoutPointer pointer)
{
    if (pointer == null) 
    {
        Debug.LogWarning("IsDrawingOverAllowed: Pointer is null!");
        return true; // or false, it depends on your design..
    }

    if (!pointer.IsValid()) // <-- I also like early returns :D 
    {
        return true;
    }

    var leadingTrailSegment = pointer.GetLeadingTrailSegment()
    return !midline.IsParallelTo(leadingTrailSegment);
}

I know that can be more 'verbose' but remember the idea that create code for machines is easy, but code for humans is harder.. At the end you want to have some code easy to read, understand and maintain.

Side Note; Yes, I know that sometimes can be useful, like in Physics.Raycast but if you are not implementing the TryParse pattern (if you want for example avoid the use of try/catch) I cannot see much gain trying to have just one method that does two things.