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

Sleeping barber problem in Erlang

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

Problem

I'm learning Erlang. It's my first time with a non-imperative programming language.

I've written a code and I want some thoughts about it:

  • Is my logic easy to understand?



  • Is my code idiomatic?



  • Is my code well organized?



  • Is my code using Erlang features correctly?



My goal here is improve readability and functional-thinking. I'm not concerned about performance.

```
-module(sb).
-export([start/0, barberShop/3, clientGenerator/2, barber/2]).
-define(MAX_BARBERS, 1).
-define(MAX_SITS, 5).

start() ->
BarberShopPID = spawn(sb, barberShop, [[], 0, yes]),
spawn(sb, clientGenerator, [BarberShopPID, 10]).

clientGenerator(BarberShopPID, ClientsToGenerate) ->
case ClientsToGenerate of
0 ->
BarberShopPID ! clientGeneratorIsDone;
_ ->
timer:sleep(random(1000, 6000)),
Hair = random(100, 600),
BarberShopPID ! {client, Hair},
clientGenerator(BarberShopPID, ClientsToGenerate-1)
end.

barberShop(Clients, Barbers, IsCGRunning) ->
receive
{client, Hair} ->
case length(Clients) of
?MAX_SITS ->
write("A client entered, but there are no available sits."),
barberShop(Clients, Barbers, IsCGRunning);
_ ->
write("A client entered and found a available sit."),
{NewC, NewB} = tryAttend([Hair | Clients], Barbers, self()),
barberShop(NewC, NewB, IsCGRunning)
end;
barberIsDone ->
{NewC, NewB} = tryAttend(Clients, Barbers-1, self()),
case shouldContinue(NewC, NewB, IsCGRunning) of
yes ->
barberShop(NewC, NewB, IsCGRunning);
no ->
write("Bye from the glorious barber shop.")
end;
clientGeneratorIsDone ->
case shouldContinue(Clients, Barbers, no) of
yes ->
{NewC, NewB} = tryAttend(Clients, Barbers, self()),
barberShop(NewC, NewB, no);
no ->
write("Bye from the glorious barber shop.")
end
end.

barber(Hair, BarberShopPID) ->
write("Barb

Solution

Overall it looks good, I would just have some minor comments. In any case please have a look at the official Ericsson guidance.

-
Use macro ?MODULE instead of repeating sb.

-
In Erlang function names are usually_with_underscores rather than camelCasing. There are some exceptions, even in OTP applications, but in general underscores help to distinguish functions from variable names, which are usually CamelCased. The same holds for atom names.

-
I would usually put a code like this: [First | Rest] = Clients directly to the function argument to make it shorter, e.g.: tryAttend([First | Rest] = Clients, Barbers, BarberShopPID) ->

-
NewC and NewB aren't very descriptive. I would rather use NClients if NewClients are too long for you.

-
When accessing the head and tail of a list the names of variables are usually H and T, unless you want a specific name for the head, in which case the tail is just T or Tail, e.g. [H|T] = Clients or [First|T] = Clients. It's a convention which you will notice when reading someone else's code.

-
When spawning process with a function that doesn't need to be exported for its own sake try to use fun, e.g.: spawn(fun() -> barber(First, BarberShopPID) end). It's better to not expose functions that are not meant to be called from outside. And if you do have to export a function like that, then it's better to put it into a separate export with an additional comment, e.g. %% Internal API.

-
Didn't you want to write MAX_SEATS rather than MAX_SITS? :)

Context

StackExchange Code Review Q#126015, answer score: 2

Revisions (0)

No revisions yet.