[newbie] refactor function?

[Carsten Schultz]

Hi!

On Tue, Feb 01, 2005 at 01:07:10PM +0400, Gaspar Chilingarov wrote:
> Hello All!
> 
> what is a correct way of writing following function (initQuota/1) -- 
> probably i got problems thinking in a functional way ;) ?
> 
> i wish to get rid of last two cases.
> 
> following function just adds to list of tuples with range descriptions 
> initial zero value, which will be changed later in application.
> 
> -cut-
> testInitQuota() ->
>   [{ single, "AAA", 0 }] = initQuota( [ { single, "AAA" } ] ),
>   [{ range, "AAA", "BBB", 0 }] =
> 	initQuota( [ { range, "AAA", "BBB" } ] ),
>   [{ single, "AAA", 0 },{ range, "AAA", "BBB", 0 }] =
> 	initQuota( [ { single, "AAA" },{ range,    "AAA", "BBB" } ] ).
> 
> initQuota([Head | QuotaList]) ->
>   lists:reverse(initQuota(Head, QuotaList, [])).
>   
> initQuota( { range, Value1, Value2 }, [Head | QuotaList], NewList ) ->
>   initQuota( Head, QuotaList, [ { range, Value1, Value2, 0 } ] ++ 
> NewList );
> initQuota( { single, Value1 }, [Head | QuotaList], NewList ) ->
>   initQuota( Head, QuotaList, [ { single, Value1, 0 } ] ++ NewList );
> initQuota( { range, Value1, Value2 }, [], NewList ) ->
>   [ { range, Value1, Value2, 0 } ] ++ NewList;
> initQuota( { single, Value1 }, [], NewList ) ->
>   [ { single, Value1, 0 } ] ++ NewList.
> -cut-

Well I would first write a specification of what the function should
do, like

initQuota(X) ->
    lists:map(fun({range, Value1, Value2}) ->
		      {range, Value1, Value2, 0};
		 ({single, Value1}) ->
		      {single, Value1}
	      end, X).

:-)

If you do not like funs like this, that would have been

initQuota(List) ->
    lists:map(fun initQuotaElement/1, List).

initQuotaElement({range, Value1, Value2}) ->
    {range, Value1, Value2, 0};
initQuotaElement({single, Value1}) ->
    {single, Value1}.

BTW, the problem with your solution seems to be that you are doing to
pattern matches at once, although they are independent of each other.
You could have written:

initQuota(Val, List, NewList) ->
    NewVal = case Val of
		 {range, Value1, Value2} ->
		     {range, Value1, Value2, 0};
		 {single, Value1} ->
		     {single, Value1, 0}
	     end,
    case List of
	[] ->
	    [NewVal | NewList];
	[Head | Tail] ->
	    initQuota(Head, Tail, [NewVal | NewList])
    end.

However, there is no need for the first argument of initQuota.  A
usual formulation along your lines would have been

initQuota(List) ->
    initQuota(List, []).

initQuota([], Acc) ->
    lists:reverse(Acc);
initQuota([{single, Val}|Tail], Acc) ->
    initQuota(Tail, [{single, Val, 0}|Acc]);
initQuota([{range, Val1, Val2}|Tail], Acc) ->
    initQuota(Tail, [{range, Val1, Val2, 0}|Acc]).

(Your formulation is equivalent to the above, but with
 initQuota(List=[_|_]) ->
	initQuota(List, []).)

HTH,

Carsten

-- 
Carsten Schultz (2:38, 33:47)
http://carsten.codimi.de/
PGP/GPG key on the pgp.net key servers, 
fingerprint on my home page.