0

I have seen code written which contains ternary operators nested more than 7 levels deep. I find it very difficult to read the code. Is it a good practice to have nested ternary operators? What do you suggest to replace these nested ternary operators with?

This is some code I came across.

var qry = from row in tflResults.AsEnumerable()
                    let _date = row.Field<DateTime>("RestoreDate")
                    let fDeadline = row.IsNull("RestorefDeadline") ? null : row.Field<DateTime?>("RestorefDeadline")
                    where checkSecResp(noResp, SecResp, row) && checkDept(noDept, Depts, row) && checkSpace(selectedWebs, row)
                        && _date >= startDate && _date <= endDate
                    group row by
                    new
                    {
                        RestoreNumber = row.Field<string>("RestoreNumber") ? row.Field<string>("RestoreNumber") : row.Field<string>("RestoreNetNumber"),
                        EDate = _date,
                    }
                        into g
                    let r = g.FirstOrDefault()
                    let RestoreNumber = r.Field<string>("RestoreNumber") ?? r.Field<string>("RestoreNetNumber")
                    let requests = from req in queryRequest
                                   where req.RestoreIntelNumber.StartsWith(RestoreNumber + ".")
                                   where req.Status != "Deleted" && req.Status != "Cancelled"
                                   select req
                    select new
                    {
                        RestoreDate = g.Key.EDate,
                        RestoreRequestDate = r.IsNull("RestoreRequestDate") ? "" : r.Field<DateTime>("RestoreRequestDate").ToLocalTime().Date,
                        RestoreNumber = RestoreNumber,
                        RestoreDraftDeadline = r.IsNull("RestoreOffDeadline") ? "" : r.Field<DateTime>("RestoreOffDeadline").ToLocalTime(),
                        RestoreSecResp = r["RestoreSecResp"],
                        RestoreOffMemberResp = r["RestoreOffMemberResp"],
                        RequestSent = requests.Count() > 0 && requests.All(req => req.Status == "Versont" || req.Status == "Ontvogen")
                                ? "Ja"
                                : requests.Any(req => req.Status == "Versont" || req.Status == "Ontvongen") ? "Wat" : "",
                        RestoreIsfized = r.IsNull("RestoreIsfized") ? false : r.Field<bool>("RestoreIsfized"),
                        IsUrgent = r.IsNull("RestoreIsUrgent") ? "" : r.Field<bool>("RestoreIsUrgent") ? "Ja" : "",
                        WorkingDaysToPrep = r["RestoreUrgency"],
                        fDeadlineForSort = r.IsNull("RestorefDeadline") ? null : r.Field<DateTime>("RestorefDeadline").ToLocalTime()
                    };
            qry = sortedField == "fDeadline"
                ? qry.Where(b => b.fDeadlineForSort == null || startfDeadline == null || endfDeadline == null || (b.fDeadlineForSort >= startfDeadline && b.fDeadlineForSort <= endfDeadline))
                : qry.Where(b => b.RestoreDate >= startDate && b.RestoreDate <= endDate);
            qry = sortedDescending
                ? qry.OrderByDescending(b => sortedField == "fDeadline" ? b.fDeadlineForSort : b.RestoreDate)
                : qry.OrderBy(b => sortedField == "fDeadline" ? b.fDeadlineForSort : b.RestoreDate);
zizi
  • 11
  • 3
  • 5
    This is completely opinioned, however I would fail a *code review* on 3 levels deep. Even 2 levels deep is getting excessive and is usually not readable in a single glance. – TheGeneral Nov 11 '21 at 05:42
  • For two I'd probably go with regular if-else block. Any deeper I'd just extract them to their own methods. – Martheen Nov 11 '21 at 05:44
  • As others mentioned its an individual choice, however, you can replace nested ternary operators with `if` statements. I think they exist for a reason. Moreover, ternary statements run same as `if` statements, they don’t run any faster. You can read more opinions on this topic here https://stackoverflow.com/questions/694814/ternary-operator-bad-or-good-practice – Venkata N Bhupathi Nov 11 '21 at 05:55
  • 1
    Note : if you asked this question with a realistic example and asked for suggestions on how to make it more readable and reduced nesting, the question would have a better chance of surviving the opinion-based close votes, and as such likely answered in seconds – TheGeneral Nov 11 '21 at 05:56
  • You can improve readability by inserting starting a new line before each `?` and ':', indenting to indicate depth. But if tests might still be easier to read. – Jeremy Lakeman Nov 11 '21 at 06:08
  • Show us an example. I bet it will be horrendous; in my eyes you answered your own question: *I find it very difficult to read the code* - you have to work on this code and maintain it, it's your problem and it actively causes you a problem, which means it's a problem. You should fix that – Caius Jard Nov 11 '21 at 06:13
  • 1
    If you have seven levels of condition checks in an OO language like C#, there is something fundamentally wrong with the class and function design. Use a switch expression, look up tables or methods. – Mayur Ekbote Nov 11 '21 at 06:13

1 Answers1

2

The "C# Coding Conventions" from Microsoft don't mention anything related to ternary operators.

The C# language reference has an example of two level nesting like this: a ? b : (c ? d : e) This example is useful because you can explicitly show the nesting part in the expression using parenthesizes. That might help your code to be more readable.

I personally don't use nested ternary operators as it doesn't look natural and thus not readable. When I have a problem that needs several levels of conditional expressions I prefer using "if, else" statements plus methods that include the condition evaluation.

You can add a method with a meaningful name like "IsInputValid", "IsConnected", "IsCorrectFormat" etc. by using such methods you hide the details of low level components and focus on the problem that the initial method solves.

dotnetdeveloper
  • 59
  • 5