Refactor my code (pretty please)

Questions / Help
1

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…

2

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
3

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
4

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