Erlang : nested cases

Question

I'm very new to Erlang. I tried to find out if a list index is out of bounds (before trying it) so i wanted to do an if clause with something like

if lists:flatlength(A) < DestinationIndex ....

I discovered that those function results cannot be used in if guards so i used case instead. This results in a nested case statement

case Destination < 1 of
    true -> {ok,NumberOfJumps+1};
    false ->
    case lists:flatlength(A) < Destination of
        true -> 
            doSomething;
        false ->
            case lists:member(Destination,VisitedIndices) of
                true -> doSomething;
                false -> 
                    doSomethingElse
            end
    end
end.

I found this bad in terms of readability and code style. Is this how you do things like that in erlang or is there a more elegant way to do this?

Thanks in advance

Answer 1

Before you take the following as some magical gospel, please note that the way this function is entered is almost certainly unidiomatic. You should seek to limit cases way before you get to this point -- the need for nested cases is itself usually a code smell. Sometimes it is genuinely unavoidable, but I strongly suspect that some aspects of this can be simplified way earlier in the code (especially with some more thought given to the data structures that are being passed around, and what they mean).

Without seeing where the variable A is coming from, I'm making one up as a parameter here. Also, without seeing how this function is entered I'm making up a function head, because without the rest of the function to go by its pretty hard to say anything for sure.

With all that said, let's refactor this a bit:

First up, we want to get rid of the one thing we know can go into a guard, and that is your first case that checks whether Destination < 1. Instead of using a case, let's consider that we really want to call two different clauses of a common function:

foo(Destination, NumberOfJumps, _, _) when Destination < 1 ->
    {ok, NumerOfJumps + 1};
foo(Destination, _, VisitedIndices, A) ->
    case lists:flatlength(A) < Destination of
        true -> doSomething;
        false ->
            case lists:member(Destination,VisitedIndices) of
               true  -> doSomething;
               false -> doSomethingElse
            end
    end.

Not too weird. But those nested cases that remain... something is annoying about them. This is where I suspect something can be done elsewhere to alleviate the choice of paths being taken here much earlier in the code. But let's pretend that you have no control over that stuff. In this situation assignment of booleans and an if can be a readability enhancer:

foo(Destination, NumberOfJumps, _, _) when Destination < 1 ->
    {ok, NumberOfJumps + 1};
foo(Destination, _, VisitedIndices, A) ->
    ALength = lists:flatlength(A) < Destination,
    AMember = lists:member(Destionation, VisitedIncides),
    NextOp =
        if
            ALength     -> fun doSomething/0;
            AMember     -> fun doSomething/0;
            not AMember -> fun doSomethingElse/0
        end,
    NextOp().

Here I have just cut to the chase and made sure we only execute each potentially expensive operation once by assigning the result to a variable -- but this makes me very uncomfortable because I shouldn't be in this situation to begin with.

In any case, something like this should test the same as the previous code, and in the interim may be more readable. But you should be looking for other places to simplify. In particular, this VisitedIndices business feels fishy (why don't we already know if Destination is a member?), the variable A needing to be flattened after we've arrived in this function is odd (why is it not already flattened? why is there so much of it?), and NumberOfJumps feels something like an accumulator, but its presence is mysterious.

What makes me feel weird about these variables, you might ask? The only one that is consistently used is Destination -- the others are only used either in one clause of foo/4 or the other, but not both. That makes me think this should be different paths of execution somewhere further up the chain of execution, instead of all winding up down here in a super-decision-o-matic type function.

EDIT

With a fuller description of the problem in hand (reference the discussion in comments below), consider how this works out:

-module(jump_calc).
-export([start/1]).

start(A) ->
    Value = jump_calc(A, length(A), 1, 0, []),
    io:format("Jumps: ~p~n", [Value]).

jump_calc(_, Length, Index, Count, _) when Index < 1; Index > Length ->
    Count;
jump_calc(Path, Length, Index, Count, Visited) ->
    NewIndex = Index + lists:nth(Index, Path),
    NewVisited = [Index | Visited],
    NewCount = Count + 1,
    case lists:member(NewIndex, NewVisited) of
        true  -> NewCount;
        false -> jump_calc(Path, Length, NewIndex, NewCount, NewVisited)
    end.

Always try to front-load as much processing as possible instead of performing the same calculation over and over. Consider how readily we can barrier each iteration behind guards, and how much conditional stuff we don't even have to write because of this. Function matching is a powerful tool -- once you get the hang of it you will really start to enjoy Erlang.