-
Notifications
You must be signed in to change notification settings - Fork 17
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
Documentation and Commenting of Project #1
base: master
Are you sure you want to change the base?
Conversation
…oved dead code for Stone.java
… function both inline and with JavaDocs in Game.java.
… function both inline where needed and with JavaDocs in Board.java.
Hi, thanks for the PR, I'll go over it shortly. |
// If white has no blotted stones the play is illegal | ||
if (barWhite == 0) | ||
return false; | ||
// If the opponent has a made then the play cannot be done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a mistake or my english is not good enough? What is "a made"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A made in backgammon is when two stones reside on a point, such that an opponent cannot land on said point.
if(stoneColors[i] == color && (i != except || stoneCounts[i] > 1)){ | ||
|
||
// If the positions representing not the colors base have a stone of | ||
// color 'color' then not all stones are in the base |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this sentence hard to understand. What about "If the non-base positions have..."?
// color 'color' then not all stones are in the base | ||
for (int i = f; i < t; i++) { | ||
// Except allows for testing of no remaining moves, if all but i are | ||
// in base, and i is an invalid move return true to notify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this comment is right.
The except
variable is used for ignoring single stone on the except
position when checking if there are some stones outside of their base. It's used for testing if I can move stone that is not in base out of the game.
|
||
// Unique ID corresponding to this exception type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serialVersionUID
is documented in https://docs.oracle.com/javase/7/docs/api/java/io/Serializable.html and as it's part of Java specification I think it's not necessary to have it commented.
@Override | ||
public int hashCode() { | ||
// Calculates a hash code value specific to each Stone using a low prime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hashCode
is common Java method, I don't think it's necessary to have it commented when the common implementation is used and it uses obvious fields.
return false; | ||
// Else compare the Stones by color, | ||
// if they match the stones are considered equivalent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly like hashCode
, the equals
method is common with common implementation and when there is nothing surprising that should a reader be pointed to I thing it's not necessary to have there comments.
I will fix the issues found after the assignment is marked, it is past the deadline to make code changes and will likely be penalized for such. After I receive a mark I can finish the above comments. |
Hi, any update on that? Do you still plan to fix the mentioned things? |
Summary of Changes:
Eclipse formatting (Ctrl + Shft + f) , JavaDocs, and inline comments were added to the files:
Other Changes:
Dices was replaced with the more proper name of "Dice".
Dead code was removed for functions that were duplicated or unused.
Accessibility modifiers were added to class fields, and many functions used privately were modified to be private functions over public ones.
Reason for Pull Request
The documented code is a result of an assignment for McMaster Universities Software Engineering Communications Course, 3I03.