CODEWITHSUNDEEP
c#refactoringternary-operator
KaptajnKold
KaptajnKold

Reputation: 10946

Nested ternary operators

I have this code:

_viewModel.PhoneDefault = user == null ? "" :
    (string.IsNullOrEmpty(user.PhoneDay) ?
        (string.IsNullOrEmpty(user.PhoneEvening) ?
            (string.IsNullOrEmpty(user.Mobile) ? "" : 
                user.Mobile) :
            user.PhoneEvening) :
         user.PhoneDay);

Is there a better way to write this to make it more readable?

Upvotes: 9

Views: 30999

Answers (7)

Vladimir
Vladimir

Reputation: 3689

Well, as long as the entries can be just null or valid:

    if (user == null)
    {
        _viewModel.PhoneDefault = string.Empty;
    }
    else
    {
        _viewModel.PhoneDefault = user.PhoneDay ?? user.PhoneEvening ?? user.Mobile ?? string.Empty;
    }

You can even define an extension method on string to convert empty strings to null and use it here, but I am just crazy.

If they can be an empty string, just use the other people answers. Jason's method is nice and clean.

Upvotes: 3

LucasMetal
LucasMetal

Reputation: 1393

Just answering because I haven't seen the approach I use in this kind of scenarios in any of the existing answers.

What about...

_viewModel.PhoneDefault = user == null ? string.Empty :
    !string.IsNullOrEmpty(user.PhoneDay)     ? user.PhoneDay :
    !string.IsNullOrEmpty(user.PhoneEvening) ? user.PhoneEvening :
    !string.IsNullOrEmpty(user.Mobile)       ? user.Mobile : 
    string.Empty;

It's inconvenient that you have to check by user nullness at the beginning because that forces to repeat string.Empty, but nevertheless I think it's pretty readable.

My two cents ;)

Upvotes: 6

Dominik Szymański
Dominik Szymański

Reputation: 822

Now, with power of LINQ, you can make it simple and readable within single line like

_viewModel.PhoneDefault = user == null ? "" :
(new [] {user.PhoneDay, user.PhoneEvening, user.Mobile}).FirstOrDefault(s => !string.IsNullOrEmpty(s)) ?? "";

Upvotes: 1

Jon49
Jon49

Reputation: 4606

Although I like Gabe's and Vladimir's answers the most. Here's a way to make nested ternaries look decipherable.

_viewModel.PhoneDefault =
  user == null
    ? ""
  : (string.IsNullOrEmpty(user.PhoneDay)
    ? (string.IsNullOrEmpty(user.PhoneEvening)
      ? (string.IsNullOrEmpty(user.Mobile)
           ? ""
         : user.Mobile)
      : user.PhoneEvening)
    : user.PhoneDay);

Granted, since you have to surround them with parentheses it makes it more difficult to do this. In JavaScript it actually comes out very elegant since it allows for nested ternaries without the parentheses. Although your situation doesn't really need this syntax sometimes this syntax comes in very handy.

Upvotes: 4

Jason Evans
Jason Evans

Reputation: 29186

Write a seperate method to get the phone number, something like this:

public string GetDefaultPhone(User user)
        {
            if(user == null)
            {
                return string.Empty;
            }

            if(!string.IsNullOrEmpty(user.PhoneDay))
            {
                return user.PhoneDay;
            }

            if(!string.IsNullOrEmpty(user.PhoneEvening))
            {
                return user.PhoneEvening;
            }

            if(!string.IsNullOrEmpty(user.Mobile))
            {
                return user.Mobile;
            }

            return string.Empty;
        }

And then in your code:

_viewModel.PhoneDefault = GetDefaultPhone(user);

Upvotes: 7

Gabe
Gabe

Reputation: 86778

In your case you can write a helper function, like this:

// return the first parameter that isn't null or empty
public static string CoalesceStrings(params string[] src)
{
    foreach (var str in src)
        if (!string.IsNullOrEmpty(str))
            return str;
    return "";
}

Then just call it, like this:

_viewModel.PhoneDefault = user == null ? "" :
    CoalesceStrings(user.PhoneDay, user.PhoneEvening, user.Mobile);

Upvotes: 14

HypnoToad
HypnoToad

Reputation: 605

Adding some more parenthesis might make it more readable. You could also break it down into a series of if/else statements

if (user != null)
{
  if (!string.IsNullOrEmpty(user.PhoneDay))
    _viewModel.PhoneDefault = user.PhoneDay;
  else if (!string.IsNullOrEmpty(user.PhoneEvening))
    _viewModel.PhoneDefault = user.PhoneEvening;
  else if (!string.IsNullOrEmpty(user.Mobile))
    _viewModel.PhoneDefault = user.Mobile;
  else
    _viewModel.PhoneDefault = "";
}

Upvotes: 0

Related Questions