CODEWITHSUNDEEP
elixir
user9106677
user9106677

Reputation: 61

Enum.reduce retruning :ok after first iteration

I am passing a list consist of integers as strings in Enum.reduce, and running a function on each value of the list. But the problem is in second iteration it is returning :ok, due to which function is giving error, I am using multiple IO.puts in my code just to trace the error.

Below is my function to_decimal and when we pass "11" as parameter, then it should return decimal representation of this binary value. But it stops in second iteration, I will write the output below - 

def to_decimal(string) do
    list = String.graphemes(string)
    len = length(list)
    Enum.reduce(list,0, fn x,acc ->
            temp=String.to_integer(x)
            power = round(:math.pow(2,len-1))
            IO.puts(power)
            acc = acc + temp*power
            IO.puts(acc)
            len=len-1
            IO.puts(len)
            end)
end

iex(1)> Binary.to_decimal("11")
2
2
1
2
** (ArithmeticError) bad argument in arithmetic expression: :ok + 2
    :erlang.+(:ok, 2)
    binary.exs:16: anonymous fn/3 in Binary.to_decimal/1
    (elixir) lib/enum.ex:1925: Enum."-reduce/3-lists^foldl/2-0-"/3

Upvotes: 1

Views: 444

Answers (3)

fhdhsni
fhdhsni

Reputation: 1629

You should return the new acc in each iteration. Currently the last expression in the "callback" function (that is the fun in thereduce(enumerable, acc, fun)) is IO.puts which returns :ok. You should build the new acc in each iteration and return it which will be used in the next iteration.

Note that in Elixir data structures are immutable. Also beware that you're not changing the len that exists in the upper scope. Basically what you doing is throw_away = len - 1.

Check out this answer if you want to know more about variable scope.

Upvotes: 3

Patrick Oscity
Patrick Oscity

Reputation: 54684

For real-world code, you should go with the pre-rolled solution:

String.to_integer(string, 2)

That being said, let's treat this as an exercise for learning more about Elixir. A common pitfall with Elixir is a misconception about variable scope. Variables in Elixir are immutable and always local in scope, but they can be rebound to new values. This makes it hard to wrap your head around if you're just starting out because it looks like you can change the value, but in fact you're just binding the variable name to a new value. This means the original scope will still be bound to the same value:

counter = 0
for i <- 1..10 do
  counter = counter + 1
end
IO.inspect counter # prints 0

Another way to think about this is that the inner counter is simply a new variable that happens to have the same name as the outer one. The above example is equivalent to:

counter = 0
for i <- 1..10 do
  new_counter = counter + 1
end

To get around this, as you already correctly observed, we use functions like Enum.reduce/2-3 which allow us to store intermediate results in an accumulator. So each of the variables that need remembering must go in the accumulator. The value for the next iteration is then returned from the anonymous function, which allows Enum.reduce to pass it back into the next iteration.

In your case, this means that you'll want to remember the len and sum, which we can put in a tuple to pass it around as {sum, len}. The basic structure of your reduction should be:

result = Enum.reduce list, {0, len}, fn {sum, len} ->
  new_sum = sum + ...
  new_len = ...
  {new_sum, new_len}
end
{sum, _} = result
sum

If you want to go one step further and get a better feeling of how all those pieces fit together, I highly recommend reading the first few chapters of Programming Elixir by Dave Thomas. It includes some exercises to build utilities like Enum.reduce from the ground up which helps tremendously.

As a final remark, there are still some things that could be improved in your code. For example it will happily accept invalid digits like "2". Just as an inspiration, here's how I would tackle this problem:

string
|> String.graphemes
|> Enum.reverse
|> Enum.with_index
|> Enum.reduce(0, fn
  {"0", _}, acc -> acc
  {"1", i}, acc -> acc + :math.pow(2, i)
end)
|> trunc()

I already hear some voices screaming: "but now you're iterating multiple times over the list" to which I would reply that clarity goes over performance optimization, as long as this is not a bottleneck. If you really find that you need to squeeze the last bit of performance out of this, it's usually best to write a tail-recursive function that does not rely on the Enum module at all, but I'll leave that up to you to explore.

Upvotes: 3

sbacarob
sbacarob

Reputation: 2212

Using Enum.reduce the way you're doing it isn't the best approach for your problem.

This is because each iteration of Enum.reduce should "return" the accumulator for the next iteration.

When you put acc = acc + temp * power and len = len-1, you aren't assigning acc and len to have the values you're giving them in the next iteration, just on the current one, and in fact, you should be getting warnings about those variables being unused.

At the end of your block, since the last thing you're doing is len = len - 1, and since len will always be the same, what you're passing on to the next iteration is always 1, in the case of the input of "11".

And when you add the IO.puts/1, at the end, you're passing :ok (the "return" of IO.puts) to the next iteration, which produces the error you're seeing.

an alternative approach using Enum.reduce could be to keep a tuple {len, sum} and at the end of the block, put always the new values for len and sum.

So, you could change it like:

def to_decimal(string) do
  list = String.graphemes(string)
  len = length(list)

  Enum.reduce(list, {0, len}, fn x, {sum, len} ->
    temp = String.to_integer(x)
    power = :math.pow(2, len - 1) |> round()
    sum = sum + temp * power
    len = len - 1

    {sum, len}
  end)
end

This will at the end return a tuple containing {n, l} where n will be the decimal representation, and, l, the final length.

Upvotes: 1

Related Questions