Erlang code critique

Question

I am trying to get my head round some basic erlang functionality and I could do with some comments on the following.

I have the following erlang code that takes a list of tuples and returns a list minus an element if a key is found:

delete(Key, Database) ->
    remove(Database, Key, []).

remove([], Key, Acc) ->
    Acc;
remove([H|T], Key, Acc) ->
    if
        element(1, H) /= Key ->             
            [H| remove(T, Key, Acc)];
        true  -> 
            remove(T, Key, Acc)
    end.

Is this a good way of doing this?

The if statement seems incorrect.

Also is my use of the accumulator Acc making this tail recursive?

Answer 1

No, your usage of Acc doesn't make it tail recursive. Your branch of if returns [H| remove(T, Key, Acc)] which is not tail call and this branch would be used most of time. To be more precise your usage of Acc is useless because it would be [] whole time, you don't change its value at all. Correct code should look like.

delete(Key, Database) ->
    remove(Database, Key, []).

remove([], Key, Acc) ->
    lists:reverse(Acc);
remove([H|T], Key, Acc) ->
    if
        element(1, H) /= Key ->             
            remove(T, Key, [H|Acc]);
        true  -> 
            remove(T, Key, Acc)
    end.

But if your list members are always pairs I would prefer direct pattern match:

delete(Key, Database) ->
    remove(Database, Key, []).

remove([], Key, Acc) ->
    lists:reverse(Acc);
remove([{Key, _}|T], Key, Acc) ->
    remove(T, Key, Acc);
% if it should delete only first occurrence then lists:reverse(Acc, T);
remove([H|T], Key, Acc) ->
    remove(T, Key, [H|Acc]).

But I think this is example where can apply Myth: Tail-recursive functions are MUCH faster than recursive functions so I would use much simpler recursive version:

delete(Key, []) -> [];
delete(Key, [{Key, _}|T]) -> delete(Key, T);
% if it should delete only first occurrence then just T;
delete(Key, [H|T]) -> [H | delete(Key, T)].