# Codewars kata : reverse and rotate

I just started to learn Elixir and to train on Codewars. I solved this kata but I'm not very satisfied of my code and I have no clue (literally) how to improve it. Recursion seems the way to go here, but I'm convinced the code can be optimized a little more.

Here is the full problem :

The input is a string str of digits. Cut the string into chunks of size sz (ignore the last chunk if its size is less than sz).

If a chunk represents an integer such as the sum of the cubes of its digits is divisible by 2, reverse it; otherwise rotate it to the left by one position. Put together these modified chunks and return the result as a string.

Could you help me please ?

defmodule Revrot do
def revrot(str, sz) do
if str == "" or sz <= 0 or String.length(str) < sz do
""
else
chunk = str |> String.slice(0,sz)
t = chunk |> String.to_integer |> Integer.digits |> Enum.reduce(0, fn(x, acc) -> (:math.pow(x,3) |> round) + acc end)
temp = case (rem t,2) do
0 -> chunk |> String.reverse
_ -> String.slice(chunk, 1, sz-1) <> String.first(chunk)
end
temp <> revrot(String.slice(str,sz..-1), sz)
end
end
end


So, the only major comment I'd have on your code is that it's a bit hard to understand what's going on. It becomes a lot easier when you have the Kata text to help, but ideally the code itself should be quite easy to understand just by looking at it.

I would split your code up into the seperate stages, rather than having a single function that does everything. It should make it much easier to see what the code is actually doing at a glance. There's also a few built in elixir functions that you can use to simplify things.

So, essentially you have three stages:

• Split the string into chunks.
• Rotate or reverse each chunk.
• Put the string back together.

I would write a function for each of these stages, then you can compose those functions to get the full process.

First, chunking. You could do this manually, but there's actually already a function that does this in Elixir called Enum.chunk. So you can convert the string into a list of it's unicode codepoints, chunk it, then join it back into a string, like so:

def chunks(str, sz) do
str |> String.to_charlist |> Enum.chunk(sz) |> List.to_string
end


Next, we'll need a function to check if we should reverse or not. You're implementation doesn't seem bad, but:

• Elixir provides Enum.sum, which is easier than implementing the sum manually with Enum.reduce.
• I would split the sum and the pow operations into two stages.
• I wouldn't be afraid to use newlines and whitespace: it can be easier to read than a single line with a lot of operations going on.

So we could write this like:

def cube_digits(chunk) do
chunk
|> String.to_integer
|> Integer.digits
|> Enum.map(fn (x) -> x |> :math.pow(3) |> round end)
end

def should_reverse?(chunk) do
(chunk |> cube_digits |> Enum.sum |> rem(2)) == 0
end


Your rotate code seemed fine, but lets pull it into a function:

def rot_left(chunk) do
String.slice(chunk, 1, -1) <> String.first(chunk)
end


Then you can put this all together into a single function, making use of the into argument to for to join everything back together again:

def revrot(str, sz) do
for chunk <- chunks(str, sz), into: "" do
if should_reverse?(chunk) do
String.reverse(chunk)
else
rot_left(chunk)
end
end
end


If you were performing some more complicated operations inside the for it might have been worth using pipes along with Enum.map & Enum.join instead of for, but for this operation I think for ends up nice and concise.