<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Steve Vinoski schreef op 31-1-2015 om
17:18:<br>
</div>
<blockquote
cite="mid:CAO+zUOV39BUCEEBLKR_7geMqb9MGAA1mX056fmY=0jNduJP1tw@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Sat, Jan 31, 2015 at 9:31 AM,
Roelof Wobben <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:r.wobben@home.nl" target="_blank">r.wobben@home.nl</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Thanks,<br>
<br>
Everything is working now.<br>
Finnaly I have solved the whole exercise like this :<br>
<br>
-module(db).<br>
<br>
-export([new/0, destroy/1, write/3, read/2, delete_key/3,
delete/2, match/2, match_key/3]).<br>
<br>
new() -><br>
[].<br>
<br>
destroy(_Db) -><br>
ok.<br>
<br>
write(Key, Element, Db) -><br>
[ {Key, Element} | Db ].<br>
<br>
read(_Key, []) -><br>
{error, "Instance"};<br>
<br>
read(Key, [Head | List]) -><br>
case element(1, Head) of<br>
Key -> {ok, element(2, Head)};<br>
_ -> read(Key, List)<br>
end.<br>
<br>
delete_key( _Key, [] , List2) -><br>
List2 ;<br>
<br>
delete_key( Key, [Head | Tail], List3 ) -><br>
case element(1, Head) of<br>
Key -> delete_key(Key, Tail, List3 );<br>
_ -> delete_key(Key, Tail, [Head | List3] )<br>
end.<br>
<br>
delete( _Key, [] ) -><br>
{error, "Instance"};<br>
<br>
delete(Key, List) -><br>
match_key (Key, List, []).<span class=""><br>
<br>
match_key( _Key, [] , List) -><br>
List ;<br>
<br>
match_key( Key, [Head | Tail], List ) -><br>
case element(1, Head) of<br>
Key -> match_key(Key, Tail, [element(2,Head) |
List] );<br>
_ -> match_key(Key, Tail, List )<br>
end.<br>
<br>
</span><span class="">
match( Key, [] ) -><br>
{error, "Instance"};<br>
<br>
match(Key, List) -><br>
</span>
match_key (Key, List, []).<br>
<br>
<br>
Now I have 2 questions :<br>
<br>
1) Is this a good solution.<br>
</blockquote>
<div><br>
</div>
<div>Does it work? If it works, then it's potentially a good
solution. But you should think about including test code
so you can refactor with confidence, and to make it easier
for others to provide the help you're seeking.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">2)
Can I somewhere hide my helper functions (match_key,
delete_key) and do not get a warning about unused.</blockquote>
<div><br>
</div>
<div>If you're getting warnings about unused functions,
there's a chance the way you're calling them doesn't match
their definitions. Assuming you're working from example
3-4 of "Erlang Programming", it wants new/0, destroy/1,
write/3, delete/2, read/2, and match/2 to comprise the
API. So let's make that change:</div>
<div><br>
</div>
<div>
<div>-export([new/0, destroy/1, write/3, delete/2, read/2,
match/2]).</div>
</div>
<div><br>
</div>
<div>Compiling your code with this change yields:</div>
<div><br>
</div>
<div>
<div>/private/tmp/db.erl:23: Warning: function
delete_key/3 is unused</div>
<div>/private/tmp/db.erl:47: Warning: variable 'Key' is
unused</div>
</div>
<div><br>
</div>
<div>Why is this delete_key/3 unused? The reason is that the
only place that calls it is the delete_key/3 function
itself. If it's not exported, then only functions in the
same module can call it, and of no other functions in the
module call it, it's unused. As pointed out previously,
the bug is here:</div>
<div><br>
</div>
<div>
<div>delete(Key, List) -></div>
<div> match_key (Key, List, []).</div>
</div>
<div><br>
</div>
<div>which should be:</div>
<div><br>
</div>
<div>
<div>delete(Key, List) -></div>
<div> delete_key (Key, List, []).</div>
</div>
<div><br>
</div>
<div>If we make that change, and add a leading underscore to
Key on line 47, the code compiles without warning. But
does it work? Let's add an exported test/0 function based
on the sample session in the book to see:</div>
<div><br>
</div>
<div>
<div>test() -></div>
<div> Db = db:new(),</div>
<div> Db1 = db:write(francesco, london, Db),</div>
<div> Db2 = db:write(lelle, stockholm, Db1),</div>
<div> {ok, london} = db:read(francesco, Db2),</div>
<div> Db3 = db:write(joern, stockholm, Db2),</div>
<div> {error, instance} = db:read(ola, Db3),</div>
<div> [joern, lelle] = db:match(stockholm, Db3),</div>
<div> Db4 = db:delete(lelle, Db3),</div>
<div> [joern] = db:match(stockholm, Db4),</div>
</div>
<div> ok.</div>
<div><br>
</div>
<div>Then run it:</div>
<div><br>
</div>
<div>
<div>5> db:test().</div>
<div>** exception error: no match of right hand side value
{error,"Instance"}</div>
<div> in function db:test/0 (/private/tmp/db.erl,
line 59)</div>
</div>
<div><br>
</div>
<div>Line 59 is:</div>
<div><br>
</div>
<div>
<div> {error, instance} = db:read(ola, Db3),</div>
</div>
<div><br>
</div>
<div>The error is a failure to match the return value of
read/2 against the expected {error, instance}. The problem
is here:</div>
<div><br>
</div>
<div>
<div>read(_Key, []) -></div>
<div> {error, "Instance"};</div>
</div>
<div><br>
</div>
<div>Let's change that:</div>
<div><br>
</div>
<div>
<div>read(_Key, []) -></div>
<div> {error, instance};</div>
</div>
<div><br>
</div>
<div>Also change the failing clauses for match/2 and
delete/2 the same way. Now recompile and rerun the test:</div>
<div><br>
</div>
<div>
<div>7> db:test().</div>
<div>** exception error: no match of right hand side value
[]</div>
<div> in function db:test/0 (/private/tmp/db.erl,
line 60)</div>
</div>
<div><br>
</div>
<div>Line 60 is:</div>
<div><br>
</div>
<div>
<div> [joern, lelle] = db:match(stockholm, Db3),</div>
</div>
<div><br>
</div>
<div>Looks like match/2 doesn't work, since it's returning
an empty list. Why? match/2 just calls match_key/3:</div>
<div><br>
</div>
<div>
<div>match_key( _Key, [] , List) -></div>
<div> List ;</div>
<div><br>
</div>
<div>match_key( Key, [Head | Tail], List ) -></div>
<div> case element(1, Head) of</div>
<div> Key -> match_key(Key, Tail, [element(2,Head) |
List] );</div>
<div> _ -> match_key(Key, Tail, List )</div>
<div> end.</div>
</div>
<div><br>
</div>
<div>The problem is the call to element(1, Head) returns the
Name element of the tuple, not the Location. You have your
calls to element(1, Head) and element(2, Head) in the
wrong places in this function. Let's swap them:</div>
<div><br>
</div>
<div>
<div>match_key( Key, [Head | Tail], List ) -></div>
<div> case element(2, Head) of</div>
<div> Key -> match_key(Key, Tail, [element(1,Head) |
List] );</div>
<div> _ -> match_key(Key, Tail, List )</div>
<div> end.</div>
</div>
<div><br>
</div>
<div>Recompile and re-test:</div>
<div><br>
</div>
<div>
<div>9> db:test().</div>
<div>** exception error: no match of right hand side value
[lelle,joern]</div>
<div> in function db:test/0 (/private/tmp/db.erl,
line 60)</div>
</div>
<div><br>
</div>
<div>The test expects [joern,lelle] but match/2 now returns
[lelle,joern]. Why? The reason is here, on line 43:</div>
<div><br>
</div>
<div>
<div> Key -> match_key(Key, Tail, [element(1,Head) |
List] );</div>
</div>
<div><br>
</div>
<div>Every time we find a match, we push the result onto the
head of our accumulator list passed to the next iteration.
That means the last result we find will be first in the
results. This is a common issue with accumulator-based
recursive functions, and normally you'd fix it by calling
lists:reverse/1 to reverse the final result at line 39:</div>
<div><br>
</div>
<div>
<div>match_key( _Key, [] , List) -></div>
<div> lists:reverse(List);</div>
</div>
<div><br>
</div>
<div>But the book says no calls to the lists module are
allowed, so we can't do this. Another approach is to
append to the accumulator list instead, at line 43:</div>
<div><br>
</div>
<div>
<div> Key -> match_key(Key, Tail, List ++
[element(1,Head)]);</div>
</div>
<div><br>
</div>
<div>With this change in place, let's recompile and rerun
the tests:</div>
<div><br>
</div>
<div>
<div>11> db:test().</div>
<div>ok</div>
</div>
<div><br>
</div>
<div>Yay, the test works. Now, can we improve the code?</div>
<div><br>
</div>
<div>One thing that sticks out are the calls to element/2.
Those can be replaced with tuple pattern matching. For
example, read/2 looks like this:</div>
<div><br>
</div>
<div>
<div>read(Key, [Head | List]) -></div>
<div> case element(1, Head) of</div>
<div> Key -> {ok, element(2, Head)};</div>
<div> _ -> read(Key, List)</div>
<div> end.</div>
</div>
<div><br>
</div>
<div>Let's get rid of those element/2 calls. The case is
simply doing a match of the first element of each tuple
against Key. We can do that right in the function head,
like this:</div>
<div><br>
</div>
<div>
<div>read(Key, [{Key,Location} | List]) -></div>
<div> {ok, element(2, Head)};<br>
</div>
<div><br>
</div>
</div>
<div>And since element(2,Head) is just Location, we can just
make it this:</div>
<div><br>
</div>
<div>
<div>read(Key, [{Key,Location} | _List]) -></div>
<div> {ok, Location};<br>
</div>
</div>
<div><br>
</div>
<div>OK, that works if Key matches the name in the head
tuple. But what if Key doesn't match? We need another
read/2 clause, originally handled by the second clause of
the case statement:</div>
<div><br>
</div>
<div>
<div>read(Key, [_Head | List]) -></div>
<div> read(Key, List).</div>
</div>
<div><br>
</div>
<div>With these changes in place, do our tests still pass?</div>
<div><br>
</div>
<div>
<div>16> db:test().</div>
<div>ok</div>
</div>
<div><br>
</div>
<div>Yep. Now, where else do we call element/2? In
delete_key/3:</div>
<div><br>
</div>
<div>
<div>delete_key( Key, [Head | Tail], List3 ) -></div>
<div> case element(1, Head) of</div>
<div> Key -> delete_key(Key, Tail, List3 );</div>
<div> _ -> delete_key(Key, Tail, [Head | List3] )</div>
<div> end.</div>
</div>
<div><br>
</div>
<div>We can change this roughly the same way we changed
read/2, like this:</div>
<div><br>
</div>
<div>
<div>delete_key(Key, [{Key,_Location} | Tail], List3)
-></div>
<div> delete_key(Key, Tail, List3);</div>
<div>delete_key(Key, [Head | Tail], List3) -></div>
<div> delete_key(Key, Tail, [Head | List3]).</div>
</div>
<div><br>
</div>
<div>and rerun the tests:</div>
<div><br>
</div>
<div>
<div>18> db:test().</div>
<div>ok</div>
</div>
<div><br>
</div>
<div>BTW, notice that delete_key/3 also reverses the result
list. For consistency, change it to use append instead,
like we did earlier for match_key/3:</div>
<div><br>
</div>
<div>
<div>delete_key(Key, [{Key,_Location} | Tail], List3)
-></div>
<div> delete_key(Key, Tail, List3);</div>
<div>delete_key(Key, [Head | Tail], List3) -></div>
<div> delete_key(Key, Tail, List3 ++ [Head]).</div>
</div>
<div><br>
</div>
<div>And the tests still pass. The other use of element/2 is
in match_key/3, so let's fix that the same way:</div>
<div><br>
</div>
<div>
<div>match_key(Key, [{Name,Key} | Tail], List) -></div>
<div> match_key(Key, Tail, List ++ [Name]);</div>
<div>match_key(Key, [_Head | Tail], List) -></div>
<div> match_key(Key, Tail, List).</div>
</div>
<div><br>
</div>
<div>The difference here from previous matches is the Key
matches element 2 rather than element 1. The tests still
pass with this change:</div>
<div><br>
</div>
<div>
<div>22> db:test().</div>
<div>ok</div>
</div>
<div><br>
</div>
<div>One problem with the code is that the write/2 function
unconditionally adds the name/location without checking to
see if name is already there. Most people can't be in
multiple locations at once, so get rid of any matching
name/location first:</div>
<div><br>
</div>
<div>
<div>write(Name, Location, Db) -></div>
<div> NewDb = case read(Name, Db) of</div>
<div> {ok,_Location} -></div>
<div> delete(Name, Db);</div>
<div> {error,instance} -></div>
<div> Db</div>
<div> end,</div>
<div> [{Name, Location} | NewDb].</div>
</div>
<div><br>
</div>
<div>We first read to see if the name is present; if so, we
delete it and used the modified db to write. If not found,
we just use the db passed in to write. And the tests still
pass with this change, which isn't too surprising since
we're not specifically testing this case.</div>
<div><br>
</div>
<div>At this stage, another improvement is to rename the
*_key functions -- there's no need for the _key suffix,
they can instead just be named delete and match because
they have arity 3, unlike their exported counterparts,
which have arity 2. The arity difference prevents them
from clashing. I would also stop using Key and Element as
parameter names and use Name and Location instead, as
they're more descriptive for the problem.</div>
<div><br>
</div>
<div>With these final changes in place, the whole refactored
module looks like this:</div>
<div><br>
</div>
<div>
<div>
<div>-module(db).</div>
<div><br>
</div>
<div>-export([new/0, destroy/1, write/3, delete/2,
read/2, match/2, test/0]).</div>
<div><br>
</div>
<div>new() -></div>
<div> [].</div>
<div><br>
</div>
<div>destroy(_Db) -></div>
<div> ok.</div>
<div><br>
</div>
<div>write(Name, Location, Db) -></div>
<div> NewDb = case read(Name, Db) of</div>
<div> {ok,_Location} -></div>
<div> delete(Name, Db);</div>
<div> {error,instance} -></div>
<div> Db</div>
<div> end,</div>
<div> [{Name, Location} | NewDb].</div>
<div><br>
</div>
<div>read(_Name, []) -></div>
<div> {error, instance};</div>
<div><br>
</div>
<div>read(Name, [{Name,Location} | _List]) -></div>
<div> {ok, Location};</div>
<div>read(Name, [_Head | List]) -></div>
<div> read(Name, List).</div>
<div><br>
</div>
<div>delete(_Name, [] , List) -></div>
<div> List;</div>
<div><br>
</div>
<div>delete(Name, [{Name,_Location} | Tail], List) -></div>
<div> delete(Name, Tail, List);</div>
<div>delete(Name, [Head | Tail], List) -></div>
<div> delete(Name, Tail, List ++ [Head]).</div>
<div><br>
</div>
<div>delete(_Name, []) -></div>
<div> {error, instance};</div>
<div><br>
</div>
<div>delete(Name, List) -></div>
<div> delete(Name, List, []).</div>
<div><br>
</div>
<div>match( _Name, [] , List) -></div>
<div> List;</div>
<div><br>
</div>
<div>match(Location, [{Name,Location} | Tail], List)
-></div>
<div> match(Location, Tail, List ++ [Name]);</div>
<div>match(Location, [_Head | Tail], List) -></div>
<div> match(Location, Tail, List).</div>
<div><br>
</div>
<div>match(_Location, []) -></div>
<div> {error, instance};</div>
<div>match(Location, List) -></div>
<div> match(Location, List, []).</div>
<div><br>
</div>
<div>test() -></div>
<div> Db = db:new(),</div>
<div> Db1 = db:write(francesco, london, Db),</div>
<div> Db2 = db:write(lelle, stockholm, Db1),</div>
<div> {ok, london} = db:read(francesco, Db2),</div>
<div> Db3 = db:write(joern, stockholm, Db2),</div>
<div> {error, instance} = db:read(ola, Db3),</div>
<div> [joern, lelle] = db:match(stockholm, Db3),</div>
<div> Db4 = db:delete(lelle, Db3),</div>
<div> [joern] = db:match(stockholm, Db4),</div>
<div> ok.</div>
</div>
</div>
<div><br>
</div>
<div>Study all these changes and make sure you understand
the reasoning behind them.</div>
<div><br>
</div>
<div>--steve</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>
Thanks, in the next few days I will study them.<br>
<br>
I do not have made any test because that topic is still not covered
in chapter 2 and 3 of the programming erlang book.<br>
<br>
Roelof<br>
<br>
</body>
</html>