HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Erlang pattern matching excersise

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
excersiseerlangpatternmatching

Problem

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.

Solution

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

Code Snippets

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

Context

StackExchange Code Review Q#58564, answer score: 2

Revisions (0)

No revisions yet.