Algorithm 1 #3
Algorithm 1 #3
Conversation
erictoguem
left a comment
erictoguem
left a comment
There was a problem hiding this comment.
Very well written and structured code. But you could improve it by properly naming your variables (for instance using row and column instead of x and y), properly formating your comment (using javadoc style for method commenting). Finally, you didn't implement numberOfOpenSites
| siteStatus = new boolean[SIZE*SIZE + 2]; | ||
| for (int i = 0; i < SIZE*SIZE; i++) { | ||
| siteStatus[i] = false; | ||
| } |
There was a problem hiding this comment.
This is not needed as the default value of boolean is false.
| private int OIL_BELOW; | ||
|
|
||
| // create N-by-N grid, with all sites blocked | ||
| public Percolation(int N) { |
There was a problem hiding this comment.
Please could you check that the input N is valid?
| if (r < 1 || r > SIZE || c < 1 || c > SIZE) { | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
You could improve this by returning directly a boolean expression.
return r >=1 && ...
|
|
||
| private void connectTwoSite(int x1, int y1, int x2, int y2) { | ||
| if (validIndex(x2, y2)) { | ||
| if (isOpen(x2, y2)) { |
There was a problem hiding this comment.
You could avoid enclosed if by using ANDed expression...
| // 1D of site connected to all sites in last row | ||
| private int OIL_BELOW; | ||
|
|
||
| // create N-by-N grid, with all sites blocked |
There was a problem hiding this comment.
Please could you use a javadoc like comment here
/** One line comment. */
| private boolean[] siteStatus; | ||
|
|
||
| // percolation experiment's grid width and height | ||
| private int SIZE; |
There was a problem hiding this comment.
This variable is not static final (constant), then shouldn't be written in capital letters... Same for the two bellow...
| public void open(int i, int j) { | ||
| if (!validIndex(i, j)) { | ||
| throw new IndexOutOfBoundsException("row or/and column index out of bounds"); | ||
| } |
There was a problem hiding this comment.
You're repeating this pattern three times, you could extract it to a checkIndex method and just call that one...
| return PercolateTest.connected(OIL_BELOW, H20_SURFACE); | ||
| } | ||
|
|
||
| // convert 2 dimensional(row, column) pair to 1 dimensional index |
There was a problem hiding this comment.
Please could you also use one line javadoc comment?
2 participants
No description provided.