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

Remove events variable assignment #135

Closed
wants to merge 4 commits into from
Closed

Remove events variable assignment #135

wants to merge 4 commits into from

Conversation

mjvcallibrity
Copy link

This PR removes a superfluous variable assignment in
Goby::WorldComman#describe_tile. A variable "events" was being assigned,
but not used in the #describe_tile function. This commit removes that
variable assignment as a matter of keeping the code clean.

This commit removes a superfluous variable assignment in
Goby::WorldComman#describe_tile. A variable "events" was being assigned,
but not used in the #describe_tile function. This commit removes that
variable assignment as a matter of keeping the code clean.
module Goby contains a private method `run_turn` that had a check at the
end of the function definition. While it is acceptable practice to use a
not operator ("!") before a function call, it can decrease the
readability of logic and increases cognitive overhead to understand the
meaning of the code. Creating a new private method `continue_game?`
makes explicit the purpose of that logic check at the end of the
`run_turn` method within the context of the game.
Add private method to enhance readability
@nskins
Copy link
Owner

nskins commented Apr 28, 2018

Hi @mjvcallibrity - thanks a lot for the pull request.

To say "A iff B" is to say that A is true if B is true, and vice versa. It's logically stronger than "A if B," which only covers the former statement. Change that part back to "iff" (or "if and only if"). You'll need to update that in your continue_game? function doc as well as the run_turn function doc.

The type specified in the parameter documentation of your continue_game? function should be String, not Input. The first word after the type must also match the parameter name (in this case "input").

Finally, you'll need to merge the 0.2.1 branch into your branch and change your pull request to target that branch. I do not accept pull requests directly to master.

@mjvezzani
Copy link

Hey,

Many thanks for giving a clear explanation on the "A iff B" statement. I (obviously) didn't know about that before. I'll also be sure to update the parameter documentation on continue_game?, and branch of the 0.2.1 branch.

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.

None yet

3 participants