Elm-format: Should single-line `if` expressions be allowed?

[gyzerok]

Following code

isPalindrome : List comparable -> Bool
isPalindrome list =
    List.map2 (,) list (List.reverse list)
        |> List.foldl
            (\( x, y ) acc -> if x == y then acc else False)
            True

turns to this

isPalindrome : List comparable -> Bool
isPalindrome list =
    List.map2 (,) list (List.reverse list)
        |> List.foldl
            (\( x, y ) acc ->
                if x == y then
                    acc
                else
                    False
            )
            True

What do you think about it guys?

[rtfeldman]

I guess this reduces to "should single-line _if-expressions_ be allowed?"

I'd be fine with that. I've used them before, and they didn't seem particularly more prone to getting out of hand than any other kind of expression.

[avh4]

I think this is worth considering. Though for the example given, I would tend to use let:

isPalindrome list =
    let
        step ( x, y ) acc =
            if x == y then
                acc
            else
                False
    in
        List.map2 (,) list (List.reverse list)
            |> List.foldl step True

I'd love to see a few more examples from real code. Are inline lambdas the main place where this would be useful?

[gyzerok]

I'm right now on a road of implementing 99 Haskell problems in Elm. And this is just a code for one of those problems. I would try to share more examples while moving through this problems.

However I really like idea with let :)

[simonewebdesign]

Here's an example from real code:

getAtPath configuration.path model.data
    |> valueToString
    |> (\str ->
            if str == "Yes" ||
               str == "No" then model.translate str else str
        )

turns to this:

getAtPath configuration.path model.data
    |> valueToString
    |> (\str ->
            if
                str
                    == "Yes"
                    || str
                    == "No"
            then
                model.translate str
            else
                str
       )

Definitely seems a bit too aggressive :)

[gyzerok]

It looks like here is no problem with single/multiline if, but rather with boolean expression formatting.

str
    == "Yes"
    || str
    == "No"

This looks weird to me.

[seanhess]

I would definitely prefer if/then to be more flexible.

[Zimmi48]

I vote for allowing single-line if then else expressions. I'm quite annoyed to get

toString points ++ " point" ++ (if points >= 2 then "s" else "")

turned into

```elm
toString points ++ " point"
++ (if points >= 2 then
"s"
else
""
)
````

There is no specific syntax for ternary conditions in Elm (as in C) so let short if then else expressions be.

[crazymykl]

Another vote for single-line if. It's much nicer in let-bindings, especially since those already make functions very tall.

---

yesno b =
    if b then "yes" else "no"

seems nicer to read than

yesno b =
    if b then
        "yes"
    else
        "no"

[avh4]

Evan has requested that if expressions have blank lines between the clauses https://github.com/avh4/elm-format/issues/393 which seems to make a single-line version of if more desirable.

[klazuka]

@avh4 any update on this? The first thing I thought after I saw the blank-line-between-if-clauses in the 0.8.0 changelog was "I hope single-line ifs" are supported.

[avh4]

I haven't had time to revisit this yet.

If people have snippets of real-world code where they would use single-line if statements if available, please post them here! There have been a couple posted already, but I think it would be useful to see more examples of what this would look like in the wild.

[klazuka]

I just went through my codebase and pulled out some examples:
https://gist.github.com/klazuka/fdd18790ce50ee291a1a210c63ef48f7

After looking at a lot of ifs in my codebase, it seems to me that the strongest case for single-line if is in places like an Html view or an inner declaration in a let/in expression. In such cases, multi-line if with a blank line before the else makes the code too ragged/disconnected/floaty, IMHO.

I was surprised that single-line looked wrong _much_ more often than I expected.

That being said, I think there's still a case for leaving the choice up to the developer. If they leave it all on a single line, then let it be, similar to how function arguments are only pushed out to separate lines if the developer breaks it up into separate lines.

[klazuka]

Here's another real-world example I just found in my codebase...

status quo:

p
    [ style
        [ ( "width", "70%" )
        , ( "padding", "6px" )
        , ( "margin-left", "-6px" )
        , ( "background-color"
          , if isEndUser then
                "#eef3ff"

            else
                "white"
          )
        ]
    ]
    [ text "..." ]

single-line variant:

p
    [ style
        [ ( "width", "70%" )
        , ( "padding", "6px" )
        , ( "margin-left", "-6px" )
        , ( "background-color", if isEndUser then "#eef3ff" else "white" )
        ]
    ]
    [ text "..." ]

Single line seems like a much better choice here.

[klazuka]

Sorry to keep piling on, but I'm upgrading elm-json-tree-view for 0.19 and ran into this one while dealing with the toString change with respect to Bool.

multi-line:

viewNodeInternal depth config node state =
    case node.value of
        TString str ->
            viewScalar css.string ("\"" ++ str ++ "\"") node config

        TFloat x ->
            viewScalar css.number (String.fromFloat x) node config

        TBool bool ->
            viewScalar css.bool
                (if bool then
                    "true"

                 else
                    "false"
                )
                node
                config

single-line:

viewNodeInternal depth config node state =
    case node.value of
        TString str ->
            viewScalar css.string ("\"" ++ str ++ "\"") node config

        TFloat x ->
            viewScalar css.number (String.fromFloat x) node config

        TBool bool ->
            viewScalar css.bool (if bool then "true" else "false") node config

[ianp]

I’d love to see elm-format get to the point where @evancz & co. could start using it on elm/core, and right now the core libs use single line lambdas and single line ifs in a few places, sometimes even both at the same time! E.g. here List.elm#L215

[ymtszw]

I had a possibly interesting experience around this.

I also wanted oneline if-then-else since switching relatively short expression was somewhat common in my codebase, especially in views (e.g. just switching color or border style based on activation state.)

But found elm-format does not allow that.
"Well, okay," I thought, discussion is going in this issue anyway. Time will resolve this.

In the mean time, let's just have this to avoid formatting:

ite : Bool -> a -> a -> a
ite condition then_ else_ =
    if condition then
        then_

    else
        else_

... and I used it casually, everywhere, everywhen I thought I wanted oneline logical switch.

However, recently I realized that this helper function actually have a significant flaw: it eliminates laziness of if-then-else!

It could be OK if both then_ and else_ above are static, immediate values determined at compile-time.
Though if either or both of them are resultants of function calls, unnecessary evaluation(s) will happen at runtime!

(To be clear:

ite condition (funcitonForThen arg1) (functionForElse arg2)

in this case both funcitonForThen AND functionForElse are evaluated.

if condition then
  functionForThen arg1
else
  funcitonForElse arg2

With this, only one of them are evaluated!)

So, allowing oneline if-then-else might have a practical "defensive" value in that it prevents someone like me from introducing such suboptimal escape hatch!

[ianp]

@ymtszw yup, you probably want your function to have this type signature instead:

its: Bool -> (() -> a) -> (() -> a) -> a
its c t e =
  if c then
    t ()
  else
    e ()

and then invoke it like

ite c (\_ -> t) (\_ -> e)

which is kind of a pain!

[andys8]

An example with elm-ui and a border on the bottom, if selected:

Current formatting

    el
        [ paddingXY 15 30
        , Border.color styles.colors.blue
        , Border.widthEach
            { top = 0
            , left = 0
            , right = 0
            , bottom =
                if isSelected then
                    4

                else
                    0
            }
        ]
    <|
        text content

Formatting I'd prefer

    el
        [ paddingXY 15 30
        , Border.color styles.colors.blue
        , Border.widthEach
            { top = 0
            , left = 0
            , right = 0
            , bottom = if isSelected then 4 else 0
            }
        ]
    <|
        text content

[holgerl]

It has been almost three years now. Can we get a conclusion on this issue?

[andys8]

My impression is that if somebody submits a pull request for this feature, it will most likely get merged.

• Richard wrote he is fine with it
• Evan is using single line if statements in elm/core
• Aaron said the feature is planned

[andys8]

Regarding the implementation:

Here is the formatting done.

https://github.com/avh4/elm-format/blob/abe730aa441794373729ca25ed5164e90a05d65c/src/ElmFormat/Render/Box.hs#L1393-L1424

Changing it to print in a single line should be easy. But I guess we want it to be context sensitive: If there are line breaks, they should stay, and if there are none, then it should be printed in single line. I think this information has to be collected when parsing and represented like it's done for other features (e.g. imports)

[andys8]

Using elm-ui and animations (css transitions):

Current

, htmlAttribute <| style "transition" "all 0.5s ease-out"
, alpha <|
    if isSelected then
        1

    else
        0.6
, moveUp <|
    if isSelected then
        2

    else
        0

Should be

, htmlAttribute <| style "transition" "all 0.5s ease-out"
, alpha <| if isSelected then 1 else 0.6
, moveUp <| if isSelected then 2 else 0

[elkowar]

any updates on this? its driving me crazy...

[rlefevre]

It seems there are enough real-world examples to justify testing an implementation, so I made one that only leads to single line expressions if there is no else if nor multi-line expressions in the if condition, then body or else body.

It seems to work quite well. The only drawback I noticed for now is that the following expression:

if if a then True else False then "a" else "b"

which was previously formatted as:

        if a then    
            True    

        else    
            False    
    then    
        "a"    

    else    
        "b"  

stays formatted as:

if if a then True else False then "a" else "b"

which might be a little confusing.

Given their rarity (the elm-format test that uses one is the only example I ever saw), I'm not sure if nested if conditions should be handled differently or not (maybe there is a way to add parentheses in this example?).

[ymtszw]

if if a then True else False then "a" else "b"

which might be a little confusing.

Inserting parens might indeed looks better.

if (if a then True else False) then "a" else "b"

However, as you probably noticed this should usually be reduced to:

if a then "a" else "b"

I wonder if there are practical cases where "nested ifs" are justified in Elm.
For example, negating a Bool value should better be written with Basics.not, instead of nesting ifs.

-- BAD
if (if a then False else True) then "a" else "b"

-- GOOD
if not a then "a" else "b"

-- GOOD; with a function call
if not (resolvesToBool a) then "a" else "b"

IMHO "nested ifs" cause less concerns in reality since they are usually weeded out in code reviews (or, possibly by linters such as elm-analyse.)

Also as I wrote this, I started to wonder should longer conditional expressions be parenthesized?

-- This:
if conditionalFunction (isActually quite complicated) andOrLong then "what should we do?" else "b"

-- Or this:
if (conditionalFunction (isActually quite complicated) andOrLong) then "what should we do?" else "b"
-- However I would prefer extracting such long conditional values to `let` bindings.

My overall impression is: just leave them without parens.

• If the conditional part is just a single token or a short enough expression, it is OK
• Even if it is longer, the syntax highlighting helps us good
• Nested ifs are assumingly rare enough
• From implementation POV, drawing a line between "long" and "short" is somewhat subjective and cumbersome, if we are to parenthesize "long" expressions.