Elixir Code Review Request

[powelli13]

This issue will act to save areas that I'd like feedback on from Code Reviews.

• The GameServer app uses GenServers to maintain the state for each kind of game, these GenServers have separate modules for the DynamicSupervisors. In an Elixir app should individual specifically modules GameServer.GameSupervisor and GameServer.TttGameSupervisor be specified as the DynamicSupervisor for different GenServers or should one supervisor module be used for all the similar GenServers?

• Is it bad form to have two separate Elixir apps both calling into each other? For example in GameServerWeb the Phoenix channel modules lookup the Player Queues via the Registry to add players, and then the GenServers in the GameServer app use Phoenix PubSub to broadcast game state updates to the Phoenix channels which subsequently push them to the client. Should this all be in one web app rather than an umbrella with two apps?

• Is there a standard convention for the terms that are sent to handle_info implementations? For example in GameServerWeb.TttGameChannel I use handle_info to receive broadcasted game updates from the GenServers that hold game state. I simply use tuples with atoms as the first element to define what type of update it is. Is this acceptable should another form be used?

• The various game channels, example ttt_game_channel.ex, use Registry.lookup to locate processes for games. Is this the recommended way considering that all the processes are on the same BEAM node?

Any feedback on these questions or any other aspects of the Elixir codebase is very much appreciated, thank you!

[nickdichev]

These are all awesome questions! Here's my take on things:

1. I think its fine to leave all the children under one supervision tree to start, and then split it up if you realize you want a different supervision strategy for some children.
2. I don't think you're actually calling between apps here? Phoenix.PubSub is a child of the GameServer application supervision tree not GameServerWeb. So when your GenServers publish to it, nothing is crossing the application boundary. You should check out Sasa Juric's blog post series about this topic and his library Boundary
3. I think its fine, and I do the same
4. Look at the :via tuple. Its a nice trick to directly interface with the GenServer API (or other OTP friends that take the :name option) when you're using Registry.

[powelli13]

@nickdichev thanks for the feedback and suggestions! I really appreciate it.