Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#321] In sumo_store add handle_info and terminate as optional callbacks #322

Merged
merged 1 commit into from
Jul 19, 2017

Conversation

cabol
Copy link
Contributor

@cabol cabol commented Jul 18, 2017

In sumo_store add handle_info and terminate as optional callbacks

@cabol cabol requested a review from elbrujohalcon July 18, 2017 17:43
@cabol cabol force-pushed the cabol.321.optional_callbacks branch 2 times, most recently from 47fa951 to 67fc74e Compare July 18, 2017 18:08
true -> Handler:handle_info(Info, HState);
false -> {noreply, State}
catch
_:Reason -> {stop, Reason, State}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of _, you should use Reason and you should not stop, but treat that as a result.

true -> Handler:terminate(Reason, HState);
false -> ok
catch
_:_ -> ok
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of _, you should use throw specifically.

@cabol cabol force-pushed the cabol.321.optional_callbacks branch from 67fc74e to 31aa801 Compare July 19, 2017 16:19
handle_info(Info, #state{handler = Handler, handler_state = HState} = State) ->
case erlang:function_exported(Handler, handle_info, 2) of
true ->
try
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking, but I thing the proper way to do this would be:

try Handler:handle_info(Info, HState) of
  Response -> handle_info_response(Response, State)

@cabol cabol force-pushed the cabol.321.optional_callbacks branch from 31aa801 to 9b4d3fc Compare July 19, 2017 16:48
@elbrujohalcon elbrujohalcon merged commit ca61dbd into master Jul 19, 2017
@cabol cabol deleted the cabol.321.optional_callbacks branch July 19, 2017 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants