[erlang-questions] Code critique please

[Zoltan Lajos Kis]

Sam Bobroff wrote:
> Zoltan Lajos Kis wrote:
>> Sam Bobroff wrote:
>>> Jarrod Roberson wrote:
>>>> is there anything that I can do to improve this code? Are the 
>>>> nested case of
>>>> the best way to do that? It was the only thing I could get to work.
>>>>   
>>> Hi Jarrod,
>>>
>>> I have another suggestion you might find interesting. I would like 
>>> to factor out the recursion in the "is_subscribed" function because 
>>> I think it's generally good to use comprehensions or folds if you 
>>> can, instead of creating your own recursion. Unfortunately I can't 
>>> see a way to do this trivially, so:
>>>
>>> Given this code:
>>>> % test to see if a dns_rr.domain is subscribed to
>>>> is_subscribed(_,[]) -> false;
>>>> is_subscribed(Dom,[S|Rest]) ->
>>>>     case lists:suffix(S,Dom) of
>>>>         true ->
>>>>             {ok,S};
>>>>         false ->
>>>>             is_subscribed(Dom,Rest)
>>>>     end.
>>> I would first write a function like this:
>>>
>>> %% first(Pred, List): Return either false or {true, Elem} where Elem 
>>> is the first element of List for which Pred(Elem) returns true.
>>> %% Pred must take one argument and return true or false.
>>> first(Pred, []) ->
>>>    false;
>>> first(Pred, [X | Xs]) ->
>>>    case Pred(X) of
>>>        true ->
>>>            {true, X};
>>>        false ->
>>>            first(Pred, Xs)
>>>    end.
>>>
>>> Then I could write is_subscribed like this:
>>>
>>> is_subscribed(Dom, SS) ->
>>>    first(fun(S) -> lists:suffix(S, Dom) end, SS).
>>>
>>> This makes the "is_subscribed" function really easy to understand 
>>> and provides "first" as a useful tool.
>>>
>>> To be honest I was quite surprised that it wasn't in the "lists" 
>>> module... maybe there is one somewhere I couldn't find.
>>>
>>> Sam.
>>>
>>>
>>>
>>> ________________________________________________________________
>>> erlang-questions mailing list. See http://www.erlang.org/faq.html
>>> erlang-questions (at) erlang.org
>>>
>> It is there inversed: lists:dropwhile/2
>>
>> is_subscribed(Dom, SS) ->
>>  case lists:dropwhile(fun(S) -> not lists:suffix(S, Dom) end, SS) of
>>    [X|_] -> {true, X};
>>    [] -> false
>>  end
>>
>> Regards,
>> Zoltan.
> I actually looked at dropwhile/2, but I didn't think it was a good 
> solution because:
>
> * It might create a copy of the tail of the list, which you don't need 
> (I don't know enough about Erlang to know if this happens or not).
> * The whole point of factoring is_subscribed/2 was to make it clearer 
> and I don't think it achieves that.
>
> So thanks for pointing that out but I still like the idea of a first/2 
> function :-)
>
> Cheers,
> Sam.
The list won't be copied, the function will only return a "pointer" into 
the middle of the list as far as I know.
I think your refactoring is merely putting stuff behind the scenes. Your 
is_subscribed only looks "clearer" because you purposefully made your 
first function return values in a way that no additional handling is 
required in is_subscribed. In other functions you will have to do that. 
Same with dropwhile.

Regards,
Zoltan.