12
\$\begingroup\$

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
\$\endgroup\$
6
\$\begingroup\$
  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.

\$\endgroup\$
8
\$\begingroup\$

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.

\$\endgroup\$
  • 3
    \$\begingroup\$ @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. \$\endgroup\$ – Eric Stein Dec 31 '14 at 18:45
  • 2
    \$\begingroup\$ 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... \$\endgroup\$ – Jeff Clayton Dec 31 '14 at 19:33
  • \$\begingroup\$ 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. \$\endgroup\$ – Steve Midgley Dec 31 '14 at 20:39
  • 2
    \$\begingroup\$ @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. \$\endgroup\$ – Mathieu Guindon Dec 31 '14 at 20:43
  • \$\begingroup\$ @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). \$\endgroup\$ – Steve Midgley Dec 31 '14 at 22:56
7
\$\begingroup\$

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 ? "*" : " ");
\$\endgroup\$
  • 2
    \$\begingroup\$ This is actually an excellent time to use the ternary operator and make it a single-line operation: System.out.print(bb ? "*" : " "); \$\endgroup\$ – Jesse C. Slicer Jan 5 '15 at 18:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.