[erlang-questions] Novice core review request

[Motiejus Jakštys]

On Sun, Nov 25, 2012 at 1:26 PM, Константин <for-friends@REDACTED> wrote:
> Hello,
>
> I’m Erlang novice and would be glad if someone could review my first
> application. I’d like to hear common advices about interprocesses
> communication and code style. I started a thread on Stackoverflow
> http://codereview.stackexchange.com/questions/18998/erlang-novice-erlang-otp-common-advices
> . To not duplicate there are more details can be found there.
>
> The code itself is here: https://bitbucket.org/ktyurin/erlarena

First of all your application needs a README. Purpose of the program,
briefly what it does and how it works.

Then it needs -spec for publicly exported functions, i.e. at least the
ones that you are expecting to be used by external callers (if any).
Ideally, for all exported functions. Once you have these, run dialyzer
(it's simple and it's worth it!), and you will (probably) be
overwhelmed by the number of bugs it will find for you. :)

A hint for type specs:

arena_map.erl:
-type map() :: [{something_i_did_not_immediatly_recognize}].
-export_type([map/0]).

-record(state, { % Internal room state
  name :: string(), % Room name. Log file will have the same name ++ ".txt"
  map :: arena:map(), % Reference to room map, process which stores
information about creature locations
  moves_performed :: non_neg_integer() = 0, % Number of moves
performed from start
  creatures = [arena_creature:creature()] % List of creatures
currently located in the room
}).

Note: my map() type is obviously wrong. But you get the idea.

A small notice about arena_map:create_string/1. Instead of folding and
appending to the end of the list, you could use an iolist - very
convenient Erlang feature for IO data construction. This blog post[1]
looks like a good introduction (didn't read it fully).

You should also decide which of the types must be public, private, and
opaque. In most cases you do not want to allow external applications
mangle with your type internals. Once you make a decision, put
appropriate type specifications and commentary in source files. Public
types are normally exported in <APP_NAME>.erl, and well documented.
Private types are exported only when they have to be referenced from
other modules, but they are "small enough" so you do not want to make
accessors for them. Read about -opaque in [2] and use it. :)

[1]: http://dev.af83.com/2012/01/16/erlang-iolist.html
[2]: http://www.erlang.org/doc/reference_manual/typespec.html

A side note: when somebody is going to make another poll for
erlangers, please include a question which asks about line length
restrictions.

P.S. Konstantin, there are some people allergic to lines with many
characters (like me). If you want *everyone* be happy reading your
code, stick to some safe neat and cozy value.

Regards,
Motiejus Jakštys