Add atm example#10
Conversation
tumdash
left a comment
tumdash
left a comment
There was a problem hiding this comment.
Код можно упростить:
- матчить элементы в строке определения функций, а не в отдельных строках
- использовать значение, где они используются inline
Кроме того, выглядит оправданным использовать map вместо record для state
| @@ -0,0 +1,216 @@ | |||
| -module(atm). | |||
There was a problem hiding this comment.
Стиль:
хидер %%%, в комментах описываем, зачем этот модуль
%%---------------------------------------------------
%%
%% This module implements logic of card reader
%%
%%----------------------------------------------------| -module(atm). | ||
| -behaviour(gen_statem). | ||
|
|
||
| -export([insert_card/1, push_button/1]). |
There was a problem hiding this comment.
Стиль:
-export хорошо группировать по разным признакам, например
export методов, которые предполагается использовать в других модулях (API)
export служебных функций (в случае behavior, которые необходимо экспортировать), что-то в виде:
%%% exported API
-export([insert_card/1, start_link/1]).
%%%internals of gen_statem
-export([init/1, terminate/1]).| deposit/3, | ||
| handle_event/4]). | ||
|
|
||
| -record(data, { |
There was a problem hiding this comment.
Заметка:
нормально определять записи (records) в боевом коде, только если это определение дальше модуля не нужно. Иначе имеет смысл сделать хидер файл, и включать его везде, где надо.
| attempts = 0 | ||
| }). | ||
|
|
||
| -spec start_link([{CardNo :: integer(), Pin :: integer(), Balance :: integer()}]) -> |
There was a problem hiding this comment.
👍
Specs очень надо для всех функций, которые планируются использовать снаружи (экспортируются как API).
| push_button(Button) -> | ||
| gen_statem:call(?MODULE, {push_button, Button}). | ||
|
|
||
| init([Cards]) -> |
There was a problem hiding this comment.
Стиль:
удобно выделять функции по следующему принципу:
%%% API
exported_fun() -> ok.
%%% internal callbacks
handle_info(A, B) ->
C = some_state_magic(B),
{noreply, B}.
%%% helpers
some_state_magic(OldState) ->
do_something(),
...
NewState. | ]} | ||
| end; | ||
| waiting_card(StateEvent, _StateContent, Data) -> | ||
| handle_event(StateEvent, {error, invalid_action}, waiting_card, Data). |
There was a problem hiding this comment.
Стиль:
разные режимы работы gen_statem, не совсем понятно зачем.
| waiting_pin(StateEvent, {push_button, enter}, Data) -> | ||
| {CardNo, Pin, _} = Data#data.current_card, | ||
| Input = Data#data.input, | ||
| case integer_to_list(Pin) =:= Input of |
There was a problem hiding this comment.
Код:
лучше работать с binary и int, чем со строками (string).
| handle_event(StateEvent, StateContent, waiting_pin, Data); | ||
| waiting_pin(StateEvent, time_is_out, Data) -> | ||
| handle_event(StateEvent, {error, time_is_out}, waiting_pin, Data); | ||
| waiting_pin(StateEvent, _StateContent, Data) -> |
There was a problem hiding this comment.
👍 хороший пример форматирования множественных function clauses
| 3 -> | ||
| NewData = get_default_data(Data), | ||
| {next_state, waiting_card, NewData, [{reply, From, StateContent}]}; | ||
| _ -> |
There was a problem hiding this comment.
Код:
этот case лучше исправить на A when A < 3, в конкретном примере - он допускает > 3, что будет ошибкой и мы пропустим её дальше. let if crash принцип говорит о том, что мы обрабатываем только то, как мы полагаем должна вести себя программа, а если случается чтото неожиданное (например, в результате ошибки или чего-то ещё data.attempts == 4 до начала выполнения этого кода, erlang упадет в этом месте с ошибкой case clause (что нормально тут, ибо мы этого не предполагали это значение в этом месте)
| handle_event({call, From}, {error, invalid_pin} = StateContent, waiting_pin, Data) -> | ||
| Attempts = Data#data.attempts + 1, | ||
| case Attempts of | ||
| 3 -> |
There was a problem hiding this comment.
Код:
опять же просится application:get_env и переменная окружения для этого параметра
2 participants
No description provided.