# Refactor my code (pretty please)

Hi.
I was solving some exercises on Programming Elixir, and would really appreciate if someone(s) can Elixir-ify my code.
The “my_abs” and “my_reverse” functions act like their built-in namesakes (verified).
I took this approach to maintain compatibility with the built-in namesake function, specifically, using a negative index value.

Thanks a bunch in advance.

``````def split([], _val), do: []
def split(lst, 0), do: {[], lst}
def split(lst, val) when is_integer(val) do
case (val > 0) do
true -> case (val <= length(lst)) do
true -> {take(lst, val), take(lst, -(length(lst) - val))}
false -> {take(lst, length(lst)), []}
end
false -> case (my_abs(val) <= length(lst)) do
true -> {take(lst, (length(lst) - my_abs(val))), my_reverse(take(my_reverse(lst), my_abs(val)))}
false -> {[], take(lst, length(lst))}
end
end
end

def take([], _val), do: []
def take([_head | _tail], 0), do: []
def take([head | tail], val) when is_integer(val) do
case (val > 0) do
true -> [head | take(tail, val - 1)]
false -> my_reverse(take(my_reverse([head | tail]), my_abs(val)))
end
end
``````

P.S. Also using this message to suggest adding a “Refactor my code” category and “refactor”/“refactoring” tag…

It appears like the code is more complex than necessary because split is implemented in terms of take.

Why not do it the other way around?

``def take(list, n), do: list |> split(n) |> elem(0)``
2 Likes

Some of the best examples of idiomatic elixir especially for these kind of problems can be found in the elixir code-base itself: https://github.com/elixir-lang/elixir/blob/master/lib/elixir/lib/enum.ex#L2325

3 Likes

How did you implement `my_reverse`? I suspect it’s the vanilla recursive way - because that’s how I initially did it. But strangely enough things didn’t really “click” for me personally until I came across a refactor that factored out a `shunt/2` function out of `reverse/1`:

``````defmodule Demo do
# https://en.wikipedia.org/wiki/Shunting_(rail)
def shunt([], tail),
do: tail;
def shunt([h|t], tail),
do: shunt(t, [h|tail])

def reverse(list),
do: shunt(list,[])

def join(front, tail),
do: shunt(reverse(front), tail)

defp concatr([], tail),
do: tail
defp concatr([hl|t], tail) when is_list(hl),
do: concatr(t, join(hl, tail))

def concat(lists, tail) do
lists
|> reverse()
|> concatr(tail)
end

defp flatten([], [], acc),
do: reverse(acc)                 # reverse accumulated result
defp flatten([], [h|t], acc),
do: flatten(h, t, acc)           # pop h off others be flattened next
defp flatten([h|t], others, acc) when is_list(h),
do: flatten(h, [t|others], acc); # push t onto others and flatten h first
defp flatten([h|t], others, acc),
do: flatten(t, others, [h|acc])  # shunt h to accumulator

def flatten(deep_list, tail),
do: flatten(deep_list, [tail], [])
end

IO.inspect(Demo.shunt([3,2,1],[4,5]))
IO.inspect(Demo.reverse([6,5,4,3,2,1]))
IO.inspect(Demo.join([1,2,3,4],[5,6,7]))
IO.inspect(Demo.concat([[1,2],[3,4,5],[6,7,8,9]], [10,11]))
IO.inspect(Demo.flatten([1, [[2,3,4], 5, [6, [7, [[8], 9]]]]], [10,11,12]))
``````
``````\$ elixir demo.exs
[1, 2, 3, 4, 5]
[1, 2, 3, 4, 5, 6]
[1, 2, 3, 4, 5, 6, 7]
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
\$
``````
1 Like