14

You are writing a boolean expression that might look like this:

team.Category == "A Team" && team?.Manager?.IsVietnamVet

public class Manager
{
    public bool IsVietnamVet { get; set; }
}

public class Team
{
    public string Category { get; set; }

    public Manager Manager { get; set; }
}

...and you get an error:

Operator '&&' cannot be applied to operands of type 'bool' and 'bool?'

What is the optimal/cleanest way to handle it?

  1. team.Category == "A Team" && (team?.Manager?.IsVietnamVet ?? false)

    Is that really readable?

  2. team.Category == "A Team" && (team?.Manager?.IsVietnamVet).GetValueOrDefault()

    It may not work in LINQ-to-Entities...

  3. team.Category == "A Team" && team?.Manager?.IsVietnamVet == true

    Would you really write if (condition == true) without any hesitation?

Are there any other options? Is it ultimately better to write:

  1. team.Category == "A Team" && team.Manager != null && team.Manager.IsVietnamVet
Improve this question
11
  • if(!(String.IsNullOrEmpty(team.Manager) && condition)) ... Commented Mar 24, 2016 at 23:10
  • 1
    @StevieV it was ment to be like this right from the start ;) Commented Mar 24, 2016 at 23:27
  • 1
    Looking at the code more carefully, the null conditional part should be team.Manager?.IsVietnamVet, i.e. no null conditional after team, since already can't be null. Commented Mar 29, 2016 at 11:40
  • 2
    "Would you really write if (condition == true) without any hesitation?" For a normal boolean, no, since it's superfluous. However, for a nullable boolean, this is relevant. nullableBool == true is basically testing for nullableBool != false && nullableBool != null (and it's this second part that makes it useful and therefore not superfluous) Commented Sep 29, 2017 at 12:54
  • 2
    I started to use nullableBool == true if I don't create a wrapper. It is readable because you normally don't write == true when using regular boolean as @Flater mentions, so it suggests that the variable is nullable. Aditionally, it improves LINQ readability because you don't use multiple null conditions as @Fabio mentions. Commented Oct 2, 2017 at 11:27

4 Answers 4

Reset to default
5

In this particular case, it might be wise to follow the Law of Demeter i.e.

public class Team
{
    public bool IsManagerVietnamVet => Manager?.IsVietnamVet ?? false;
}

More generally, if a boolean expression is complex or ugly then there's nothing to say you couldn't divide it (or part of it) into a separate statement:

bool isVietnamVet = Manager?.IsVietnamVet ?? false;

if (team.Category == "A Team" && isVietnamVet)

When stepping through code in the debugger, it's often nicer to have elaborate conditions packaged up into a single bool just to save a little bit of mouseover-hovering; in fact, it might just be nicer to put the whole thing in a bool variable.

bool isVietnamVetAndCategoryA = (team.Category == "A Team"
    && Manager?.IsVietnamVet ?? false);

if (isVietnamVetAndCategoryA)

or with LINQ:

var wibble = from flight in airport
             from passenger in flight.manifest
             let isOnPlane = 
                 (flight.FinishedBoarding && passenger.Flight == flight.FlightNumber)
             where !isOnPlane
             select passenger;
Improve this answer
5
  • I think I am mostly leaning towards such a solution. Commented Mar 29, 2016 at 4:50
  • 1
    New C# syntax to save a few lines: public bool IsManagerVietnamVet => (team.Category=="") && (team.Manager?.IsVietnamVet?? false); Commented Mar 29, 2016 at 12:19
  • 1
    Just keep in mind that long a && b && c &&...are executed sequentially, and the next one is not even executed if the previous one is false. So people usually put fastest at the beginning. Keep that in mind when moving stuff into a separate bool-var Commented Apr 8, 2019 at 21:31
  • @jitbit It's more about maintainability. Micro-optimisations such as the one you mention are generally unnecessary - it's not worth worrying about unless performance is identified as an issue and profiling shows that making changes like that would have any noticeable impact. Commented Apr 9, 2019 at 6:19
  • @BenCottrell I would call it a "micro" optimization if the IsManagerVietnamVet property goes to check in a database ;) Commented Apr 9, 2019 at 14:36
6

I think option 3 (i.e. == true) is the cleanest way to test that a bool? is true, because it's very clear about what it does.

In most code x == true doesn't make sense, because it's the same as x, but that does not apply here, so I think == true wouldn't be very confusing.

Improve this answer
3
  • 1
    I think this would definitely be the way to go if we wanted to use the null conditional operator, because it is also linq safe. The question is whether the readability is good. From my point of view, the choice is between opt 3 and opt 4 and I would choose 4 over 3 for the readability reason, also the programmer's intention seems a bit clearer. I have marked Ben Cottrell's answer as correct because I like that approach a bit more. If I could mark two answers, I would. Commented Mar 29, 2016 at 4:57
  • 1
    @Santhos -- re "the programmer's intention seems a bit clearer". IMHO, this is a case where the first time a programmer sees a?.b == true they will be confused, but once they grasp what it is doing, it becomes a very readable idiom, much nicer than having to test != null in the middle of a complex expression. Same for a?.b ?? false, which seems to have gained traction as the most popular solution, because it matches what you would type if you were dealing with a type other than boolean [though I don't like it for boolean; for me, it doesn't read as naturally; I prefer == true]. Commented Mar 13, 2018 at 1:10
  • @Santhos, 4 is not thread-safe (Manager can potentially be modified between the checks) so should be avoided whenever possible. 3 is objectively better (except that the first ? should be removed as you already accessed team unconditionally). Commented Nov 22, 2024 at 1:55
3

Expanding on Ben Cottrell's answer, the "Null Object" pattern can help you further.

Instead of returning a null team/manager, extract ITeam and IManager interfaces and return meaningful alternative implementations:

public class NoManager : IManager
{
    public bool IsVietnamVet => false;
}

public class NoTeam : ITeam
{
    public bool ManagedByVietnamVet => false;

    public IManager Manager => new NoManager();
}

Then all of a sudden you can do team.ManagedByVietnamVet safely.

This of course relies on the upstream provider of team to be null-safe - but that can be ensured with appropriate testing.

Improve this answer
0
-4

I wrote a simple class you could use:

 public class MyBool 
    {
        public bool? Value { get; set; }

        public MyBool(bool b)
        {
            Value = b;
        }

        public MyBool(bool? b)
        {
            Value = b;
        }

        public static implicit operator bool(MyBool m)
        {
            return m?.Value ?? false;
        }

        public static implicit operator bool?(MyBool m)
        {
            return m?.Value;
        }

        public static implicit operator MyBool(bool m)
        {
            return new MyBool(m);
        }

        public static implicit operator MyBool(bool? m)
        {
            return new MyBool(m);
        }

        public override string ToString()
        {
            return Value.ToString();
        }
    }

If reliable to use a custom type, of course. You can compare MyBool with both bool and Nullable<bool>.

Improve this answer
1
  • 1
    This site is for questions about software design rather than how to get specific pieces of code to work, so if you believe this is the best solution then your answer should focus on why you believe a class like this is the best possible solution, not the exact code needed to implement it. Commented Mar 27, 2016 at 1:24

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.