jfmengels / elm-review-simplify

Provides elm-review rules to simplify your Elm code
https://package.elm-lang.org/packages/jfmengels/elm-review-simplify/latest/
BSD 3-Clause "New" or "Revised" License
20 stars 9 forks source link

`case`-ing when `Maybe.withDefault` will do #139

Closed SiriusStarr closed 1 year ago

SiriusStarr commented 1 year ago

You would know better than I if there are appreciable performance benefits to the case, in which case this should probably be avoided, but I've encountered this pattern quite a bit

case expression of
    Just x ->
        x

    Nothing ->
        y

which is easily replaceable by the more clear and concise Maybe.withDefault y (expression)

lue-bird commented 1 year ago

Feels more like a code-style change to me, similar to how elm-analyze reports Nothing -> Nothing cases.

On the performance side, the default value will in practice usually cheap to eagerly compute. But also some times not, especially with "else handle the rest" like in recursion.

firstJust : NonEmptyList (Maybe a) -> Maybe a
firstJust ( head, tail ) =
    case head of
        Just headContent -> headContent
        Nothing ->
            List.Extra.findMap identity tail
-->
firstJust : NonEmptyList (Maybe a) -> Maybe a
firstJust ( head, tail ) =
    Maybe.withDefault (List.Extra.findMap identity tail) head

where the tail is always searched, even when the head is already Just. (Also not sure I like this rule myself in general, even from a code understandability side.)

SiriusStarr commented 1 year ago

Ehhh, I forgot Maybe.withDefault isn't lazy. Closing this, though I'd still like a solution to flag this for trivial cases.