Abwas submitting percolation assignment#2
Conversation
erictoguem
left a comment
erictoguem
left a comment
There was a problem hiding this comment.
Good code, but how you name your variables, properly formating your code, or even using the proper javadoc style comment could improve it.
Please do address my comments...
| { | ||
| checkBounds(a, b); | ||
| int x = b; | ||
| int y = a; |
There was a problem hiding this comment.
You don't need these variables. Why don't you just name method parameters a and b to x and y?
| if (b > gridLength || b < 1) | ||
| { | ||
| throw new IndexOutOfBoundsException("column index b out of bounds"); | ||
| } |
There was a problem hiding this comment.
This code could be better formatted.
| isOpen[virtualBottomIndex] = true; /// open virtual bottom site | ||
|
|
||
| percolation = new WeightedQuickUnionUF(arraySize); | ||
| fullness = new WeightedQuickUnionUF(arraySize); |
There was a problem hiding this comment.
fullness doesn't contain bottom virtual site, then could be represented with arraySiye -1 elements.
| for (int b = 1; b <= n; b++) | ||
| { | ||
| /// connect all top row sites to virtual top site | ||
| int a = 1; |
| int indexToLeft = siteIndex(a, b - 1); | ||
| percolation.union(siteIndex, indexToLeft); | ||
| fullness.union(siteIndex, indexToLeft); | ||
| } |
There was a problem hiding this comment.
You're repeating this pattern 4 times. You could instead create a "private void connect(int siteIndex, int row, int col)" method where you could first check row and column are valids and that the cell is open, before creating the connection...
| public boolean isFull(int a, int b) | ||
| { | ||
| int siteIndex = siteIndex(a, b); | ||
| //return (percolation.connected(virtualTopIndex,siteIndex) && isOpen[siteIndex]); |
There was a problem hiding this comment.
Please could you remove this comment, it's not needed at all...
| } | ||
| else { | ||
| return isOpen[siteIndex(1,1)]; | ||
| } |
There was a problem hiding this comment.
You could have used the ternary operator here instead of the if else...
| private double[] thresholdResults; | ||
|
|
||
| private int T; | ||
| // perform T independent computational experiments on an n-by-n grid |
There was a problem hiding this comment.
This is not a well formatted javadoc comment and please could you place it closer to PercolationStats
| // converts between two dimensional coordinate system and site array index. | ||
| // throws exceptions on invalid bounds. valid indices are 1 : n^2 | ||
| // a is the row; b is the column | ||
| private int siteIndex(int a, int b) |
There was a problem hiding this comment.
naming your variables row and column instead of a and b will make your code much more readable. Please could you update this code to use row and column instead of a and b?
| // holds each experiment's percolation threshold result | ||
| private double[] thresholdResults; | ||
|
|
||
| private int T; |
There was a problem hiding this comment.
non final static fields in java shouldn't be capitabilized. Please could you call this one trial instead?
| public void open(int row, int col) // open site (row, col) if it is not open already | ||
| public boolean isOpen(int row, int col) // is site (row, col) open? | ||
| public boolean isFull(int row, int col) // is site (row, col) full? | ||
| public int numberOfOpenSites() // number of open sites |
There was a problem hiding this comment.
You didn't implement this one... Please could you implement it?
2 participants
No description provided.