csharpier icon indicating copy to clipboard operation
csharpier copied to clipboard

Strange nested ternary formatting in edge case

Open stone-w4tch3r opened this issue 10 months ago • 5 comments

Formatted code:

using System.Linq;

public class FormattingTest
{
    public static void NotOkNestedFormatting()
    {
        var steps = (Step[])[new(), new()];
        var test =
            steps[^1].Type.ToString().Contains('x') ? 100
            : steps.ToList().LastOrDefault(x => x is { Type: StepType.Failure }) is Step s ? s.Count
            : 1;
    }
}

public class Step
{
    public StepType Type { get; set; }

    public int Count { get; set; }
}

public enum StepType
{
    Success,
    Failure,
}

Expected behavior:

var test =
        steps[^1].Type.ToString().Contains('x') 
                ? 100
                : steps.ToList().LastOrDefault(x => x is { Type: StepType.Failure }) is Step s 
                        ? s.Count
                        : 1;

Notes:

This seems to be some sort of edge case. Simple nested ternary works fine for me.

P.S. thanks for awesome tool! It's a pity that c# missed opinionated formatting for so long

stone-w4tch3r avatar Mar 17 '25 15:03 stone-w4tch3r

That is the expected format of ternaries. The goal is for a chained ternary to format similiar to a switch expression, because it logically is very similar. The example below is of course a best case, where everything is a similar size vs the real world where your code isn't similar in size. Prettier also adopted this formatted, but hid it behind a flag.

var languageCode =
    something.IsNotBlank() ? something
    : otherThing.IsNotBlank() ? otherThing
    : thingThing.IsNotBlank() ? thingThing
    : "enUs";

var languageCode = someValue switch
{
    something => something,
    otherThing => otherThing,
    thingThing => thingThing,
    _ => "enUs",
};

I'm calling the above a chained ternary because you can also do this, which I call a nested ternary. Which is much harder for me to wrap my head around.

return firstCondition
    ? secondCondition
        ? firstValue
        : secondValue
    : thirdCondition
        ? thirdValue
        : fourthValue;

And an extreme example (we do have a ternary about this big in our work codebase). If this was indented again at each level it becomes incredibly hard to read. Running into this was what motivated the newish nested ternary formatting.

var parameterType =
    property.PropertyType == typeof(string) ? "string"
    : property.PropertyType == typeof(int) ? "int"
    : property.PropertyType == typeof(int?) ? "int?"
    : property.PropertyType == typeof(DateTimeOffset) ? "DateTimeOffset"
    : property.PropertyType == typeof(DateTimeOffset?) ? "DateTimeOffset?"
    : property.PropertyType == typeof(DateTime) ? "DateTime"
    : property.PropertyType == typeof(DateTime?) ? "DateTime?"
    : property.PropertyType == typeof(decimal) ? "decimal"
    : property.PropertyType == typeof(decimal?) ? "decimal?"
    : property.PropertyType == typeof(Guid) ? "Guid"
    : property.PropertyType == typeof(Guid?) ? "Guid?"
    : property.PropertyType == typeof(bool) ? "bool"
    : property.PropertyType == typeof(bool?) ? "bool?"
    : property.PropertyType == typeof(byte[]) ? "byte[]"
    : property.PropertyType == typeof(float) ? "float"
    : property.PropertyType.Name;

belav avatar Mar 19 '25 19:03 belav

Interesting. I see now why it worked for other ternaries.

Maybe we can add a feature toggle or sort of formatting-rule-comment for this case? You see, when you have switch-like ternary with a single rule, as in your example, this formatting is fine. But it can be replaced with usual switch statement/expression =)

In my case conditions are independent, and csharpier makes it harder to read this code block.

I am ready to open a PR, if it will not take more than a day of work, but please list a couple of files that I will need to look at

stone-w4tch3r avatar Mar 20 '25 14:03 stone-w4tch3r

CSharpier's philosophy doesn't agree with feature toggles or formatting rule comments. You could //csharpier-ignore the line in question to control the formatting directly.

One possibility is having csharpier break a ternary if it sees the left side as "complex", something like this. Where complex is probably something like "more than 3 property accesses or method invocations". There are similar rules in other places that break before line length is hit.

The code to change is here if you wanted to take a stab at it.

       var test =
            steps[^1].Type.ToString().Contains('x')
                ? 100
            : steps.ToList().LastOrDefault(x => x is { Type: StepType.Failure }) is Step s 
                ? s.Count
            : simpleCall() ? someValue
            : longMethodCallsDoThisToo(parameter__________________________________________)
                ? someValue
            : 1;

belav avatar Mar 20 '25 18:03 belav

It can take a second to get used to a few CSharpier differences over personal opinion - but trust the process and stick with it. The ternary formatting is pretty good - and the consistency across developers wins in the long run.

For what it is worth we have a nested ternary 3x the size of the one you provided :D

Hona avatar Mar 20 '25 22:03 Hona

You see, when you have switch-like ternary with a single rule, as in your example, this formatting is fine.

FWIW, I also dislike this style of nested ternary. But the above comments made me realize I should just change my code to use a switch expression. (so I still end up avoiding CSharpier's current nested ternary style)

SamuelT-Beslogic avatar Sep 25 '25 17:09 SamuelT-Beslogic