3
\$\begingroup\$

I recently started my first project with Erlang. I've been skimming documentation and various books to get the information I need, and it's going well, but now that I have something that works I want to go "back to the basics" to fill in some holes in my knowledge, and more importantly, my style. So I took a look at some exercises in the book "Programming Erlang", and this one struck me as one I could implement and post here to get feedback on one topic I feel I'm implementing "sub-optimally" to say the least.:

-module(scratch).
-author("Oak").

%% API
-export([new/0,destroy/1,write/3,read/2,match/2]).

new()-> {db,[]}.

destroy({db,_Data})->{db,[]}.

write(Key,Element,{db,Data}) -> {db,[{Key,Element}|Data]}.

read(_Key,{db,[]})-> error(no_instance);
read(Key,{db,[{CurKey,CurValue}|_Data]}) when Key =:= CurKey-> CurValue;
read(Key,{db,[{_CurKey,_CurValue}|Data]}) -> read(Key,{db,Data}).

match(_Element,{db,[]})->[];
match(Element,{db,[{CurKey,CurValue}|Data]}) when Element =:= CurValue -> [CurKey | match(Element,{db,Data})];
match(Element,{db,[{_CurKey,_CurValue}|Data]}) -> match(Element,{db,Data}).

I'd like to know if the pattern matching I'm using is in line with the style that's expected when coding in Erlang. I could think of multiple ways of doing this, but this is what comes "naturally" to me right now, I'm not sure how elegant it is. I'm looking for tips about style more than the actual "algorithm", but if there are any glaring mistakes feel free to point them out.

\$\endgroup\$
2
\$\begingroup\$

I see one possible mistake in your code: you do not check if a key already exists before writting a new {Key,Value} pair. This may be intentional, but in this case I think that the read(Key,Db) should return a list of value rather than a single value.

Looking at pattern matching I think that you can simplify the code like this:

read(_Key,{db,[]})-> {error,no_instance};
read(Key,{db,[{Key,CurValue}|_Data]}) -> {ok,CurValue};
% the pattern maching can be done directly, no need to use when.
% I propose to return a tuple {ok,Value} to be consistent with the previous case
% so it will be possible to use pattern matching when using the result of read/2,
% even if the value returned is {error,no_instance}!
read(Key,{db,[_Pair|Data]}) -> read(Key,{db,Data}).
% don't need to create 2 variables

match(Element,Db) -> match(Element,Db,[]).
% transform the function to be tail recursive

match(_Element,{db,[]},R)->R;
match(Element,{db,[{CurKey,Element}|Data],R}) -> match(Element,{db,Data},[CurKey|R]);
% the pattern maching can be done directly, no need to use when.
match(Element,{db,[_Pair|Data]},R) -> match(Element,{db,Data},R).
% don't need to create 2 variables
\$\endgroup\$
  • \$\begingroup\$ Thanks for the review, I completely missed that I could go ahead and forgo using when clauses here, I was working with some code that required it and I guess the patter got "stuck" in my head, it's much cleaner to make the direct match. I understood how I could make the function tail recursive, but according to the author of book, some-what contrary to what other sources I had looked at, the optimization isn't necessary. He cites optimizations in new Erlang releases as making it more a matter of style now, would you agree with that? \$\endgroup\$ – Selali Adobor Jul 31 '14 at 8:10
  • \$\begingroup\$ Also on the subject of duplication of keys, I was replicating the original question, which allows for it, and is somewhat ambiguous about the behavior of read. I don't think it's meant to be a dictionary or database (even though it's referred to as one) as much as it's meant to be a list and functions that manipulate a list for practice \$\endgroup\$ – Selali Adobor Jul 31 '14 at 8:13
  • \$\begingroup\$ Well I am not an expert of erlang compiler optimizations. I use to write tail recursive functions because I am confident that it will limit process stack problems, And I find the transformation quite natural. In this particular case, I am not sure that the cons [Value|recusive call] can be built before the recursive call is achieved, but I can imagine many optimization to limit the growth of the stack and the multiple data copies, so you are probably right, it is more a matter of style. \$\endgroup\$ – Pascal Jul 31 '14 at 8:29
  • \$\begingroup\$ To complet my previous comment, I am convince that in the given example, the tail recursion does not bring bad impact because I used [V|R] construct, so there is no copy of the list needed. That would not be the case writting R ++ [V] \$\endgroup\$ – Pascal Jul 31 '14 at 8:39
  • \$\begingroup\$ I'm sure there's no downside, I'm just wondering, more objectively, which is more common, and more subjectively, which "looks prettier" \$\endgroup\$ – Selali Adobor Jul 31 '14 at 8:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.