ASCII Turtle Graphics

Question

In my college Java course, I wrote an ASCII Turtle Graphics. My professor said the only thing he could suggest was to turn the turtle into a class and create an instance of it (I hadn't because we hadn't covered classes yet). What do you think?

package javaapplication74;
import java.util.Scanner;
import static java.lang.Integer.parseInt;
public class JavaApplication74 {
// create Scanner for input
static Scanner in = new Scanner(System.in);
// enumerator of commands
private enum Commands {RESET, UP, DOWN, RIGHT, LEFT, FORWARD, DISPLAY, ERASE, REVERSE, POSITION, STOP, ERROR};
// enumerator of directions turtle can face
private enum Direction {UP, DOWN, RIGHT, LEFT};
// array of valid commands
static final char[] commands = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'r' };
// width and height of room;
static final int ROOM = 20;
// turtle's position
static int[] position = {0, 0};
// number of squares to move
static int move = 0;
// Write a "turtle graphics" program
public static void main(String[] args)
{
 // array of drawn lines
 boolean[][] floor = new boolean[ROOM][ROOM];
 // store command
 Commands command;
 // store current direction - default right
 Direction direction = Direction.RIGHT;
 // pen in drawing condition
 boolean draw = false;
 boolean erase = false;
 // provide directions
 directions();
 // get and execute commands until stop
 do {
 // get command
 command = inputCommand();
 // analyse and execute command
 switch(command) {
 case RESET:
 reset(floor);
 case UP:
 draw = false;
 erase = false;
 break;
 case DOWN:
 draw = true;
 erase = false;
 break;
 case RIGHT:
 case LEFT:
 direction = turn(command, direction);
 break;
 case FORWARD:
 floor = move(floor, direction, draw, erase);
 break;
 case DISPLAY:
 display(draw, floor);
 break;
 case ERASE:
 draw = false;
 erase = true;
 break;
 case REVERSE:
 reverse(floor);
 break;
 case POSITION:
 showPosition(direction);
 break;
 case STOP:
 case ERROR:
 break;
 } // end switch
 } while(command != Commands.STOP);
 // end do-while
} // end method main
// Provide directions
public static void directions()
{
 System.out.println("This is a turtle graphics program.");
 System.out.println("You have 11 commands available, use the number to the right to use that command:");
 System.out.printf("%s\n%s\n%s\n%s\n%s\n%s\n%s\n%s\n%s\n%s\n%s\n", "0: Clear floor.", "1: Pen up, eraser up (default position).", "2: Pen down.", "3: Turn right.", "4: Turn left.",
 "5: Go forward x spaces (enter 5,x)", "6: Display current layout", "7: Erase", "8: Show turtle's position", "9: Quit program", "r: Draw or erase current square.");
 System.out.println("Note: the turtle will draw one more symbol than the number of spaces entered\n\tbecause it will draw in the square it is initially in.");
} // end method main
// Input command
public static Commands inputCommand()
{
 // store command as string
 String command;
 // input command
 do {
 System.out.print("Enter your command: ");
 command = in.next();
 } while(!valid(command.charAt(0)));
 // end do-while
 // return command as enum value
 switch(command.charAt(0)) {
 case '0':
 return Commands.RESET;
 case '1':
 return Commands.UP;
 case '2':
 return Commands.DOWN;
 case '3':
 return Commands.RIGHT;
 case '4':
 return Commands.LEFT;
 case '5':
 // recover from bad input
 if(command.substring(1).isEmpty() || command.charAt(1) != ',' || command.substring(2).isEmpty())
 return Commands.ERROR;
 move = parseInt(command.substring(2));
 return Commands.FORWARD;
 case '6':
 return Commands.DISPLAY;
 case '7':
 return Commands.ERASE;
 case '8':
 return Commands.POSITION;
 case '9':
 return Commands.STOP;
 case 'r':
 return Commands.REVERSE;
 default:
 return Commands.ERROR;
 } // end switch
} // end method inputCommand
// Validate command
public static boolean valid(char c)
{
 for(int ch : commands) {
 if(c == ch)
 return true;
 } // end for
 // if reach this point, not valid
 return false;
} // end method valid
// Turn turtle
public static Direction turn(Commands c, Direction d)
{
 // turn turtle
 switch(d) {
 case UP:
 if(c == Commands.RIGHT)
 d = Direction.RIGHT;
 else
 d = Direction.LEFT;
 break;
 case RIGHT:
 if(c == Commands.RIGHT)
 d = Direction.DOWN;
 else
 d = Direction.UP;
 break;
 case DOWN:
 if(c == Commands.RIGHT)
 d = Direction.LEFT;
 else
 d = Direction.RIGHT;
 break;
 case LEFT:
 if(c == Commands.RIGHT)
 d = Direction.UP;
 else
 d = Direction.DOWN;
 break;
 } // end switch
 return d;
} // end method turn
// move turtle
public static boolean[][] move(boolean[][] floor, Direction d, boolean draw, boolean erase)
{
 // get direction to move turtle
 switch(d) {
 // move turtle up
 case UP:
 for(; move > 0; move--) {
 // draw, erase, or leave square
 draw(draw, erase, floor);
 // change position
 position[0]--;
 // return to caller if turtle hits wall
 if(position[0] <= 0) {
 // reset value
 position[0] = 0;
 // print warning
 System.out.println("Your turtle hit the wall and had to stop.");
 // stop moving turtle
 break;
 } // end if
 } // end for
 break;
 // move turtle down
 case DOWN:
 for(; move > 0; move--) {
 // draw, erase, or leave square
 draw(draw, erase, floor);
 // change position
 position[0]++;
 // make sure turtle does not walk through wall
 if(position[0] >= ROOM) {
 // reset value
 position[0] = ROOM-1;
 // print warning
 System.out.println("Your turtle hit the wall and had to stop.");
 // stop moving turtle
 break;
 } // end if
 } // end for
 break;
 // move turtle right
 case RIGHT:
 for(; move > 0; move--) {
 // draw, erase, or leave square
 draw(draw, erase, floor);
 // change position
 position[1]++;
 // make sure turtle does not walk through wall
 if(position[1] >= ROOM) {
 // reset value
 position[1] = ROOM-1;
 // print warning
 System.out.println("Your turtle hit the wall and had to stop.");
 // stop moving turtle
 break;
 } // end if
 } // end for
 break;
 // move turtle left
 case LEFT:
 for(; move > 0; move--) {
 // draw, erase, or leave square
 draw(draw, erase, floor);
 // change position
 position[1]--;
 // make sure turtle does not walk through wall
 if(position[1] <= 0) {
 // reset value
 position[1] = 0;
 // print warning
 System.out.println("Your turtle hit the wall and had to stop.");
 // stop moving turtle
 break;
 } // end if
 } // end for
 break;
 } // end switch
 // draw, erase, or leave square
 draw(draw, erase, floor);
 return floor;
} // end method move
// Draw, erase, or leave square
public static boolean[][] draw(boolean draw, boolean erase, boolean[][] floor)
{
 // if draw, turtle cannot leave square without drawing
 if(draw)
 floor[position[0]][position[1]] = true;
 // if erase, turtle cannot leave square without erasing
 if(erase)
 floor[position[0]][position[1]] = false;
 return floor;
} // end method draw
// Display array
public static void display(boolean draw, boolean[][] array)
{
 // loop through array
 for(boolean b[] : array) {
 for(boolean bb : b) {
 // draw picture
 if(bb)
 System.out.print("*");
 else
 System.out.print(" ");
 } // end inner for
 // return at end of row
 System.out.println();
 } // end outer for
} // end method display
// Reverse value (draw or erase square) of current position
public static void reverse(boolean floor[][])
{
 floor[position[0]][position[1]] = !floor[position[0]][position[1]];
} // end method reverse
// Show turtle's position
public static void showPosition(Direction d)
{
 // x-y coordinate
 System.out.printf("(%d, %d)\n", position[1], position[0]);
 // direction
 System.out.print("You are facing ");
 switch(d) {
 case UP:
 System.out.println("up.");
 break;
 case DOWN:
 System.out.println("down.");
 break;
 case RIGHT:
 System.out.println("right.");
 break;
 case LEFT:
 System.out.println("left.");
 break;
 } // end switch
} // end method showPosition
// Reset floor to blank
public static boolean[][] reset(boolean[][] floor)
{
 for(boolean b[] : floor)
 for(int i = 0; i < b.length; i++)
 b[i] = false;
 return floor;
} // end method reset
} // end class JavaApplication74

Answer 1

1. You should be using the Command pattern instead of having methods for each command type. This will give you separation of concerns, undo/redo functionality, and a cleaner separation of code. You'll need one interface and 11 implementations. You could also do it in a busy enum, but the separate classes are probably better.
2. You should have one model class that contains the board state - pen up, pen down, eraser down, turtle facing, and what squares are drawn on.
3. As @Mat'sMug said, almost all of your comments are a waste. In those few cases where they provide extra information about a variable, the variable should be renamed - ROOM_DIMENSION instead of ROOM, turtleFacing instead of direction, etc.
4. The mapping of input characters to commands should be exactly that, a Map<Character, Command>.
5. turtlePosition should be a Point instead of an int[].
6. Writing directly to System.out is extremely limiting. Better would be a wrapper class that accepts messages and writes them to the console. You can put that behind an interface so that other implementations (GUI) could be provided also.

What you've got is pretty typical for a class assignment. It's not unreasonable in that context, but it's not an exemplar of quality professional work. It does everything it needs to do, and it's reasonably straightforward, but it's somewhat verbose, not extensible, and not unit-testable.

Answer 2

Way too many comments. They should teach proper use of comments in school, instead of just saying "put comments everywhere!".

Good comments should say why. Comments that say what simply shouldn't exist.

This is probably the worst:

} // end method main

The comment basically says "this is a scope-ending brace".

Single-letter identifiers aren't all that great:

public static void showPosition(Direction d)

I would have named it direction, and showPosition shouldn't be responsible for getting the string for the direction - there should be a function dedicated to that, and the string returned shouldn't include the punctuation / dot from the "you are facing ..." sentence - you have mixed concerns here.

• Eric Stein: @Hosch250 No, that's not what he's saying. Comments are sometimes necessary. They should explain the reason an implementation decision was made so that the next person to edit the code understands something that would otherwise be confusing.
• Jeff Clayton: When you are working for a company, you will do well to comment properly. Make your code readable, so commenting is not always needed, but because more than one user edits, effective commentation is necessary. Don't overdo, but assume a programmer will have to work with your code later who has not seen it before. It will also help you get jobs to have it...
• Steve Midgley: I often put comments like // class Main at the end of classes or long methods where the top and the bottom of the method are usually off-screen from each other. It helps keep track of which end block is which. So I don't think it's sound general advice to say never put a comment on the end of a block of code. Otherwise I agree with everyone's input that comments should document why and not what.
• Mathieu Guindon: @steve in C# / Visual Studio, when you place the caret on a scope-ending brace and the opening brace is off the screen, a tooltip shows the line of code that opens the scope. I'd be surprised that no Java IDE does that. Also if your scopes are too large and/or confusingly nested, perhaps there's a refactoring opportunity being missed? // end of class xyz is noise if you have 1 class per code file.
• Steve Midgley: @Mat'sMug Totally true for Java - which to be fair was the OP's reference language. But in other languages, such a comment (IMO) can be handy. I write a lot in Ruby and Python and my editor (Sublime) doesn't tooltip the bottom end block. Agree that refactor makes sense for long methods but long modules or classes are inevitable (again IMO).

Answer 3

I should keep my style the same in all arrays:

// enumerator of directions turtle can face
private enum Direction {UP, DOWN, RIGHT, LEFT};
// array of valid commands
static final char[] commands = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'r' };

The top array does not have a beginning/ending space between the brace and the content, while the second does.

I should always have braces around my ifs and loops:

if(bb)
 System.out.print("*");
else
 System.out.print(" ");

Becomes:

if (bb) {
 System.out.print("*");
}
else {
 System.out.print(" ");
}

Also, most Java programmers would style that with the else on the same line as the ending if brace:

if (bb) {
 System.out.print("*");
} else {
 System.out.print(" ");
}

In fact, @Jesse C. Slicer says this would make a good ternary:

System.out.print(bb ? "*" : " ");

• Jesse C. Slicer: This is actually an excellent time to use the ternary operator and make it a single-line operation: System.out.print(bb ? "*" : " ");