patternjavaMinor
Design feedback for automaton to draw a checkerboard pattern
Viewed 0 times
drawdesignpatterncheckerboardforfeedbackautomaton
Problem
I've just completed the 3rd task of the 1st assignment offered by Stanford's programming methodologies (see the full resources for it here too).
I just need some helpful feedback on how my code can be better (decomposition-wise) and whether or not my code is comprehensible and readable.
I'm also unsure about the pre-conditions and post-conditions. Can someone please elaborate more on that? Is a pre-condition basically where the program is at when attempting the method and the post-condition just a result of the actual method?
I need all the criticism and feedback I can get.
```
/* File: CheckboardKarel.java
* ---------------------------
* This program will basically go through an entire row and land checkers on every 2nd cell.
* This process will be alternated on each row.
*/
import stanford.karel.*;
public class CheckerboardKarel extends SuperKarel {
public void run() {
while (facingWest() || facingEast()) {
if (frontIsBlocked()) {
turnKarelNorth();
} //end if
transverseRow();
transverseEvenRow();
} //end while
}
/*
* Method: turnKarelNorth
* ----------------------
* Pre-condition: The program is in a 8x1 (1 column) world and Karel has no choice but to turn north
* Post-condition: This makes Karel turn left and face north
*/
private void turnKarelNorth() {
putBeeper();
turnLeft();
moveUpwards();
}
/*
* Method: moveUpwards
* ----------------------
* This makes Karel lay down the beepers every 2 moves
*/
private void moveUpwards() {
while (facingNorth()) {
move();
move();
putBeeper();
}
}
/*
* Method: transverseRow
* ----------------------
* Pre-Condition:Karel is facing east and is ready to place the beeprs on the first odd row
* Post-Condition: Karel places all the beepers every 2 steps on
I just need some helpful feedback on how my code can be better (decomposition-wise) and whether or not my code is comprehensible and readable.
I'm also unsure about the pre-conditions and post-conditions. Can someone please elaborate more on that? Is a pre-condition basically where the program is at when attempting the method and the post-condition just a result of the actual method?
I need all the criticism and feedback I can get.
```
/* File: CheckboardKarel.java
* ---------------------------
* This program will basically go through an entire row and land checkers on every 2nd cell.
* This process will be alternated on each row.
*/
import stanford.karel.*;
public class CheckerboardKarel extends SuperKarel {
public void run() {
while (facingWest() || facingEast()) {
if (frontIsBlocked()) {
turnKarelNorth();
} //end if
transverseRow();
transverseEvenRow();
} //end while
}
/*
* Method: turnKarelNorth
* ----------------------
* Pre-condition: The program is in a 8x1 (1 column) world and Karel has no choice but to turn north
* Post-condition: This makes Karel turn left and face north
*/
private void turnKarelNorth() {
putBeeper();
turnLeft();
moveUpwards();
}
/*
* Method: moveUpwards
* ----------------------
* This makes Karel lay down the beepers every 2 moves
*/
private void moveUpwards() {
while (facingNorth()) {
move();
move();
putBeeper();
}
}
/*
* Method: transverseRow
* ----------------------
* Pre-Condition:Karel is facing east and is ready to place the beeprs on the first odd row
* Post-Condition: Karel places all the beepers every 2 steps on
Solution
Overall, your code looks pretty good! I'd say if you want a more thorough code review of your abilities, you should write up some code that doesn't simply call these external methods. It's hard to get a good feel of whether you know good programming/Java habits when all of your methods only call other methods that have been written for you.
With that said, my primary qualm is with your method names. In general, you should always design your methods with the idea that someone else will be using them in the future. That means they should be as self-explanatory and intuitive as possible.
For example:
As a developer, I would assume that the
For this reason, I would also extract the
Consistency is a big concern as well with method names. You have
To follow Java convention, any method that begins with "is" should return a
Everything I've said applies to most of your code. So to summarize, when designing methods:
Also, as a side note, I think you meant traverse rather than transverse.
With that said, my primary qualm is with your method names. In general, you should always design your methods with the idea that someone else will be using them in the future. That means they should be as self-explanatory and intuitive as possible.
For example:
private void turnKarelNorth() {
putBeeper();
turnLeft();
moveUpwards();
}As a developer, I would assume that the
turnKarelNorth() method did exactly that -- turned her North. But it doesn't actually do that explicitly; you only know that it will always turn her North because of the specific problem presented in the PDF. What it actually does is turn her left, which may or may not be North-facing depending on her current state.For this reason, I would also extract the
moveUpwards() call from this method. You can't guarantee when it's called that she will actually be facing "upwards", which again makes your design a bit inconsistent and much less flexible. Also, the method name turnKarelNorth() implies a directional adjustment, not a positional adjustment. In other words, a developer would assume that this just shifted where Karel was facing rather than actually moving her to another grid location.Consistency is a big concern as well with method names. You have
turnKarelNorth(), but then you have moveUpwards(). Choose one: either cardinal directions or up-down/left-right, but don't mix and match. It makes your code that much harder to follow.private void isFrontClear() {
if (frontIsClear()) {
move();
} // end if
else if (frontIsBlocked()) {
placeBeeperOnNextRow();
} // end else
}To follow Java convention, any method that begins with "is" should return a
boolean. This is a standard which again allows for consistency across applications and across the language as a whole. Again, the name implies getting a piece of information, not actually affecting anything (as you do when you call move()).Everything I've said applies to most of your code. So to summarize, when designing methods:
- Make sure the method name reflects what the method actually does;
- Make your method as flexible as possible to make your API more powerful and prevent the need for complete rewrites in the future;
- Have your method do one thing, and do it well;
- Be as consistent as you possibly can within your API; and
- Follow Java naming conventions
Also, as a side note, I think you meant traverse rather than transverse.
Code Snippets
private void turnKarelNorth() {
putBeeper();
turnLeft();
moveUpwards();
}private void isFrontClear() {
if (frontIsClear()) {
move();
} // end if
else if (frontIsBlocked()) {
placeBeeperOnNextRow();
} // end else
}Context
StackExchange Code Review Q#38741, answer score: 4
Revisions (0)
No revisions yet.