Speedup and simplify removeAt implementation

[janjelinek]

This is nothing major, but I found this when I optimised some my code with removeAt usage and tried custom implementation.

I did few benchmarks and this shorter implementation is faster in most cases. It's slowly only for out of range action when index is bigger than list length.

Here some results for different browsers: Chrome

Firefox

Safari

Here is benchmark code I used (only part)

longList =
    List.range 1 10000

suite : Benchmark
suite =
    describe "removeAt in List.Extra"
        [ B.compare "removeAt on short list"
            "orignial List.Extra.removeAt"
            (\() -> List.removeAt 3 [ 1, 2, 3, 4, 5 ])
            "Short implementation"
            (\() -> removeAt 3 [ 1, 2, 3, 4, 5 ])
        , B.compare "removeAt on long list"
            "orignial List.Extra.removeAt"
            (\() -> List.removeAt 666 longList)
            "Short implementation"
            (\() -> removeAt 666 longList)
        , B.compare "removeAt invalid negative value"
            "orignial List.Extra.removeAt"
            (\() -> List.removeAt -666 longList)
            "Short implementation"
            (\() -> removeAt -666 longList)
        , B.compare "removeAt out of range possitive value"
            "orignial List.Extra.removeAt"
            (\() -> List.removeAt 100000 longList)
            "Short implementation"
            (\() -> removeAt 100000 longList)
        ]

[pzp1997]

This looks good! I wonder how the following implementation would perform:

removeAt : Int -> List a -> List a
removeAt index list =
    if index < 0 then
        list
    else
        case List.drop index list of
            [] ->
                list

            _ :: rest ->
                List.take index list ++ rest

My intuition is that it will have similar performance to your optimized version without making a sacrifice when the index is larger than the length of the list. Would you mind benchmarking it against the original implementation and your optimized implementation and sharing the results?

[janjelinek]

Good point, I'll try it.

[janjelinek]

I did several measurements and clearly your suggested version with case wins for positive values out of range. So I changed this PR. Comparing both new implementations gave similar results for other case, but for this particular case (positive out of range) is significant difference. Even between old and this "case" version.

I did more benchmark with different order of cases (old first vs new first) because I noticed some bias here. Usually second case in benchmark has better results.

[Chadtech]

Cool!

I am assuming this is ready to merge? Thank you for contributing @janjelinek !

[janjelinek]

Well, it's ready to be merged, but I don't have power to do it @Chadtech :/