[erlang-questions] Request feedback on example project

[Kostis Sagonas]

On 02/03/2016 07:00 AM, zxq9 wrote:
> Two days ago I wrote a simple UUID utility as an example of bit syntax.
> Yesterday I decided I wanted it to be an example of documentation and
> readability as well.
>
> I would like feedback from new and old Erlangers in terms of readability,
> documentation, what is easy to understand, what is hard to understand, etc.

Some comments concerning the specs that you have written.

In zuuid_man.erl, I see:

-spec init(term()) -> {ok, term()}.
init(_) ->
     {ok, #s{}}.

and my immediate reaction is: well, this function does not return a 
ok-pair with any term in the second position; instead, it returns an 
#s{} record in that element.  The question that comes to my mind next, 
is why did the author of the code write that?  Does (s)he simply want to 
hide the return structure?  This is not the proper way to do this.

Further down I see:

-spec v1(State) -> {UUID, NewState}
     when State    :: #s{},
          UUID     :: zuuid:uuid(),
          NewState :: #s{}.

why not introduce a state() type for #s{} and use it throughout this 
module?  Also, if you want other modules to not be able to inspect/rely 
on the structure of the state record, declare it as opaque.


Small type lies also exist in zuuid.erl.  The function:

-spec read_uuid_string(list()) -> uuid() | bad_uuid.
read_uuid_string(UUID) ->
     Parts = string:tokens(UUID, "{-}"),
     case lists:map(fun length/1, Parts) of
          ...

clearly is not meant to accept any list; it's expecting a string().
While at it, I suggest to use a list comprehension instead of lists:map.

Similar changes need to happen in read_map_string/1 further below.


Finally, it's debatable whether it's a good idea for functions to return 
error values (e.g. 'bad_mac') instead of just crashing on inputs they 
are not supposed to handle, but I guess this is a matter of design.


Hope these help,

Kostis