views:

192

answers:

4

I'm trying to learn Erlang using the Karate Chop Kata. I translated the runit test supplied in the kata to an eunit test and coded up a small function to perform the task at hand.

-module(chop).
-export([chop/2]).
-import(lists).
-include_lib("eunit/include/eunit.hrl").
-ifdef(TEST).
chop_test_() -> [
    ?_assertMatch(-1, chop(3, [])),
    ?_assertMatch(-1, chop(3, [1])),
    ?_assertMatch(0,  chop(1, [1])),
 ....several asserts deleted for brevity...
].
-endif.

chop(N,L) -> chop(N,L,0);
chop(_,[]) -> -1.
chop(_, [],_) -> -1;
chop(N, L, M) ->
    MidIndex = length(L) div 2,
    MidPoint = lists:nth(MidIndex,L),
    {Left,Right} = lists:split(MidIndex,L),
    case MidPoint of 
    _ when MidPoint < N -> chop(N,Right,M+MidIndex);
    _ when MidPoint =:= N -> M+MidIndex;
    _ when MidPoint > N -> chop(N,Left,M)
    end.

Compiles ok.Running the test however gives, (amongst others) the following failure:

::error:badarg
 in function erlang:length/1
  called as length(1)
 in call from chop:chop/3

I've tried different permutations of declaring chop(N,[L],M) .... and using length([L]) but have not been able to resolve this issue. Any suggestions are welcome.

ps. As you might have guessed I'm a nube when it comes to Erlang.

A: 

The function erlang:length/1 returns the length of a list.

You called length(1) and 1 isn't a list.

length([1]) would return 1 length([1,2,3,4[) would return 4 etc, etc...

Gordon Guthrie
+3  A: 

So I'm pressed for time at the moment, but the first problem I see is that

chop(N,L) -> chop(N,L,0);
chop(_,[]) -> -1.

is wrong because chop(N,L) will always match. reverse the clauses and see where that gets you.

Beyond that, in the case of the 1 element list, nth(0, [1]) will fail. I feel like these lists are probably 1-indexed.

Ben Hughes
A: 

It appears that combining the remarks from Ben Hughes solves the problem. Just for completeness I'm pasting the tests-passing implementation of my binary search below.

chop(_,[]) -> -1;
chop(N,L) -> 
    Array = array:from_list(L),
chop(N,Array, 0, array:size(Array)-1).
chop(N, L, K, K) -> 
    Element = array:get(K,L),
    if 
        Element == N -> K; 
        true -> -1
    end;
chop(_, _, K, M) when M < K -> -1;
chop(N, L, K, M) ->
    MidIndex = K + ((M - K) div  2),
    MidPoint = array:get(MidIndex,L),
    case MidPoint of 
        N -> MidIndex;
        _ when MidPoint < N -> chop(N,L,MidIndex+1,M);
        _ -> chop(N,L,K,MidIndex-1)
    end.
Bas Bossink
Nice exercise, you made accidentally O(N*logN) solution for problem which should be solved in O(N) when trying applicate O(logN) method to wrong data structure.
Hynek -Pichi- Vychodil
Previous comment is now obsolete due to rewrite. But I'm certain Hynek can still find some improvements to be made. ;-).
Bas Bossink
A: 

As most significant thing to learn you should realize, that using binary search for lists in erlang is wrong idea, because lists:nth/2 is not O(1) but O(N) operation. Try list_to_tuple/1 and than do it on tuple. It is much more worth work.

It can also be worth to try it on array module.

Hynek -Pichi- Vychodil