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

Simplifications for Array #174

Open jfmengels opened 11 months ago

jfmengels commented 11 months ago

Seems like we have no simplifications for any Array functions. Don't know how I missed that because there's no work nor issues on that :sweat_smile:

Leave a comment if I missed something :)


(I'll probably move this to another issue when I have time) We could also do simplifications across types:

-- Since we do
Array.repeat 3 x
  |> Array.length
--> 3

-- we could also do
Array.repeat 3 x
  |> Array.toList
  |> List.length
--> 3
lue-bird commented 11 months ago

A super nice set of checks (I love the get and length checks).

Maybe

Array.fromList []
--> Array.empty

array |> Array.toIndexedList |> List.map Tuple.second
--> array |> Array.toList

Array.repeat 1 x
--> Array.fromList [ x ]

And for Array.slice I think we can just copy the fixes we already have for String.slice which also allows negative indexes

jfmengels commented 11 months ago

I was a little torn on

Array.repeat 1 x
--> Array.fromList [ x ]

but I guess it is a lot more idiomatic, so let's do it, yes. I'll also add

Array.initialize 1 f
--> Array.fromList [ f x ]
lue-bird commented 11 months ago

I think you meant

Array.initialize 1 f
--> Array.fromList [ f 0 ]
lue-bird commented 11 months ago

The suggested check

Array.append (Array.repeat n x) (Array.repeat m x)
--> Array.repeat (n + m) x (if n and m are literals, add them up directly)

Only works if we know them as literals and neither is negative. E.g.

let m = -1
in
Array.append (Array.repeat 2 x) (Array.repeat m x)
--> Array.repeat 0 x

would be wrong. (The correct result in this case is Array.repeat 2 x)

Equally,

Array.push x (Array.repeat n x)
--> Array.repeat (n + 1) x (if n is a literal, just increment it)

Only works if we know n as a non-negative literal. E.g.

let n = -1
in
Array.push x (Array.repeat n x)
--> Array.repeat 0 x

would be wrong. (The correct result in this case is Array.push x Array.empty)

jfmengels commented 11 months ago

All good points :+1: Should we also do Array.push x Array.empty --> Array.fromList [ x ]? It's more idiomatic, though I suspect the former to be faster, but also by only a tiny amount. (And by that logic we should not fix 1 :: [] to [ 1 ])

jfmengels commented 11 months ago

Should we simplify Array.length (Array.repeat n x) to max 0 n? It's not the prettiest fix but it's quite a bit simpler and definitely more performant.

jfmengels commented 11 months ago

I added Array.repeat n x |> Array.toList -> List.repeat n x

jfmengels commented 10 months ago

I added Array.get n (Array.repeat n x) -> Nothing (and same for initialize)

morteako commented 6 months ago
array |> Array.toIndexedList |> List.map Tuple.first to List.range 0 (List.length array - 1)

should probably be

array |> Array.toIndexedList |> List.map Tuple.first to List.range 0 (Array.length array - 1)