3
\$\begingroup\$

This question is an extension of this one, which is my implementation of a coordinate system using the Java learning software Karel J. Robot. With the below code, I created a way to automatically make the robot place beepers on spots on the coordinate plane to imitate drawing letters.

Here is the code that is the basis of the coordinate system, adapted from the linked question above:

//This class creates methods to govern the basic movements of the robot.
//The robot moves on a Cartesian plane with origin (1,1) and turns at 90˚ angles.
//Created 02/08/2016 by Trevor B. (Stack Exchange user Eridan)
//Last updated 02/18/2016

import kareltherobot.Robot;

public class BetterRobot extends Robot {
    public int xCoordinate;
    public int yCoordinate;
    public Direction currentDirection;
    public int beeperCount;
    public String robotName;

    public BetterRobot(String name, int avenue, int street, Direction direction, int beepers) {
        super(street, avenue, direction, beepers);

        robotName = name;
        xCoordinate = avenue;
        yCoordinate = street;
        currentDirection = direction;
        beeperCount = beepers;
    }

    //MOVEMENT

    public void move(int steps) {
        for(int i = 0; i < steps; i++) {
            if(frontIsClear()) {
                super.move();

                if(facingNorth()) {
                    yCoordinate++;
                }
                else if(facingSouth()) {
                    yCoordinate--;
                }
                else if(facingEast()) {
                    xCoordinate++;
                }
                else if(facingWest()) {
                    xCoordinate--;
                }
            }
            else {
                System.out.println("Error shutoff: Ran into wall");
                turnOff();
                break;
            }
        }
    }

    public void move() {
        move(1);
    }

    public void leftTurns(int turns) {
        for(int i = 0; i < turns; i++) {
            super.turnLeft();
        }
    }

    public void turnLeft() {
        leftTurns(1);
    }

    public void turnRight() {
        leftTurns(3);
    }

    public void turnAround() {
        leftTurns(2);
    }

    //DIRECTIONS

    public void faceNorth() {
        while(!facingNorth()) {
            turnLeft();
        }
        currentDirection = North;
    }

    public void faceSouth() {
        while(!facingSouth()) {
            turnLeft();
        }
        currentDirection = South;
    }

    public void faceEast() {
        while(!facingEast()) {
            turnLeft();
        }
        currentDirection = East;
    }

    public void faceWest() {
        while(!facingWest()) {
            turnLeft();
        }
        currentDirection = West;
    }

    //COORDINATE CHANGE

    public void coordinateChange(int xDestination, int yDestination) {
        int xDifference = xDestination - xCoordinate;
        int yDifference = yDestination - yCoordinate;

        if(xDifference > 0) {
            faceEast();
        }
        else if(xDifference < 0) {
            faceWest();
        }
        move(Math.abs(xDifference));

        if(yDifference > 0) {
            faceNorth();
        }
        else if(yDifference < 0) {
            faceSouth();
        }
        move(Math.abs(yDifference));
    }

    //BEEPER MANAGEMENT

    public void putBeeper(int drops) {
        for(int i = 0; i < drops; i++) {
            if(anyBeepersInBeeperBag()) {
                super.putBeeper();
            }
            else {
                System.out.println("Error shutoff: Not enough beepers in bag");
                turnOff();
                break;
            }
        }
        beeperCount -= drops;
    }

    public void pickBeeper(int drops) {
        for(int i = 0; i < drops; i++) {
            if(nextToABeeper()) {
                super.pickBeeper();
            }
            else {
                System.out.println("Error shutoff: Not next to a beeper");
                turnOff();
                break;
            }
        }
        beeperCount -= drops;
    }

    public void putAllBeepers() {
        while(anyBeepersInBeeperBag()) {
            putBeeper();
        }
    }

    public void pickAllBeepers() {
        while(nextToABeeper()) {
            pickBeeper();
        }
    }

    public void putBeeperAndMove() {
        putBeeper();
        move();
    }

    public void pickBeeperAndMove() {
        pickBeeper();
        move();
    }

    public void beeperToASCII() {
        int beepersPickedUp = 0;

        while(nextToABeeper()) {
            pickBeeper();
            beepersPickedUp++;
        }

        System.out.println((char) beepersPickedUp);
    }

    //ACQUIRING INFORMATION ABOUT ROBOT STATE

    public String getRobotName() {
        return robotName;
    }

    public int getXCoordinate() {
        return xCoordinate;
    }

    public int getYCoordinate() {
        return yCoordinate;
    }

    public Direction getCurrentDirection() {
        return currentDirection;
    }

    public int getBeeperCount() {
        return beeperCount;
    }

    public void printRobotInformation() {
        if(beeperCount!=infinity) {
            System.out.printf("Information about the robot %s \nLocation: (%d, %d) \nFacing: %s \nBeepers: %d", getRobotName(), getXCoordinate(), getYCoordinate(), getCurrentDirection(), getBeeperCount());
        }
        else {
            System.out.printf("Information about the robot %s \nLocation: (%d, %d) \nFacing: %s \nBeepers: Infinity", getRobotName(), getXCoordinate(), getYCoordinate(), getCurrentDirection());
        }
    }
}

Here is the code that extends the above to enable drawing letters.

//This class creates methods to draw characters on the plane.
//In the grids, 1 represents a beeper, and 0 represents empty space
//Created 02/18/2016 by Trevor B. (Stack Exchange user Eridan)
//Last updated 02/18/2016

public class LetterRobot extends BetterRobot {

    public LetterRobot(String name, int avenue, int street, Direction direction, int beepers) {
        super(name, avenue, street, direction, beepers);
    }

    public void drawLetter(int distanceToTop, String letterGrid) {
        int firstXCoordinate = xCoordinate;
        int firstYCoordinate = yCoordinate;
        int maximumXCoordinate = xCoordinate;

        move(distanceToTop);

        for(int numberIndex = 0; numberIndex < letterGrid.length(); numberIndex++) {
            if(letterGrid.charAt(numberIndex) == '0') {
                faceEast();
                move(1);
                maximumXCoordinate = Math.max(maximumXCoordinate, xCoordinate);
            }
            else if(letterGrid.charAt(numberIndex) == '1') {
                faceEast();
                putBeeperAndMove();
                maximumXCoordinate = Math.max(maximumXCoordinate, xCoordinate);
            }
            else if(letterGrid.charAt(numberIndex) == ' ') {
                coordinateChange(firstXCoordinate, yCoordinate-1);
            }
            else if(letterGrid.charAt(numberIndex) == 'n') {
                coordinateChange(maximumXCoordinate + 1, firstYCoordinate);
                faceNorth();
            }
        }
    }

    public void drawString(String string) {
        for(int characterIndex = 0; characterIndex < string.length(); characterIndex++) {
            char character = string.charAt(characterIndex);

            switch(character) {

            case 'A': //41
                drawLetter(4, "0110 "
                            + "1001 "
                            + "1111 "
                            + "1001 "
                            + "1001n");
                break;

            case 'B': //42
                drawLetter(4, "11110 "
                            + "10001 "
                            + "11110 "
                            + "10001 "
                            + "11110n");
                break;

            case 'C': //43
                drawLetter(4, "0111 "
                            + "1000 "
                            + "1000 "
                            + "1000 "
                            + "0111n");
                break;

            case 'D': //44
                drawLetter(4, "1110 "
                            + "1001 "
                            + "1001 "
                            + "1001 "
                            + "1110n");
                break;

            case 'E': //45
                drawLetter(4, "1111 "
                            + "1000 "
                            + "1110 "
                            + "1000 "
                            + "1111n");
                break;

            case 'F': //46
                drawLetter(4, "1111 "
                            + "1000 "
                            + "1110 "
                            + "1000 "
                            + "1000n");
                break;

            case 'G': //47
                drawLetter(4, "01111 "
                            + "10000 "
                            + "10111 "
                            + "10001 "
                            + "01111n");
                break;

            case 'H': //48
                drawLetter(4, "1001 "
                            + "1001 "
                            + "1111 "
                            + "1001 "
                            + "1001n");
                break;

            case 'I': //49
                drawLetter(4, "11111 "
                            + "00100 "
                            + "00100 "
                            + "00100 "
                            + "11111n");
                break;

            case 'J': //4A
                drawLetter(4, "0001 "
                            + "0001 "
                            + "0001 "
                            + "1001 "
                            + "0110n");
                break;

            case 'K': //4B
                drawLetter(4, "1001 "
                            + "1010 "
                            + "1100 "
                            + "1010 "
                            + "1001n");
                break;

            case 'L': //4C
                drawLetter(4, "1000 "
                            + "1000 "
                            + "1000 "
                            + "1000 "
                            + "1111n");
                break;

            case 'M': //4D
                drawLetter(4, "11111 "
                            + "10101 "
                            + "10101 "
                            + "10101 "
                            + "10001n");
                break;

            case 'N': //4E
                drawLetter(4, "10001 "
                            + "11001 "
                            + "10101 "
                            + "10011 "
                            + "10001n");
                break;

            case 'O': //4F
                drawLetter(4, "01110 "
                            + "10001 "
                            + "10001 "
                            + "10001 "
                            + "01110n");
                break;

            case 'P': //50
                drawLetter(4, "1110 "
                            + "1001 "
                            + "1110 "
                            + "1000 "
                            + "1000n");
                break;

            case 'Q': //51
                drawLetter(4, "011100 "
                            + "100010 "
                            + "100010 "
                            + "100010 "
                            + "011111n");
                break;

            case 'R': //52
                drawLetter(4, "1110 "
                            + "1001 "
                            + "1110 "
                            + "1010 "
                            + "1001n");
                break;

            case 'S': //53
                drawLetter(4, "0111 "
                            + "1000 "
                            + "0110 "
                            + "0001 "
                            + "1110n");
                break;

            case 'T': //54
                drawLetter(4, "11111 "
                            + "00100 "
                            + "00100 "
                            + "00100 "
                            + "00100n");
                break;

            case 'U': //55
                drawLetter(4, "1001 "
                            + "1001 "
                            + "1001 "
                            + "1001 "
                            + "0110n");
                break;

            case 'V': //56
                drawLetter(4, "10001 "
                            + "10001 "
                            + "01010 "
                            + "01010 "
                            + "00100n");
                break;

            case 'W': //57
                drawLetter(4, "10001 "
                            + "10101 "
                            + "10101 "
                            + "10101 "
                            + "11111n");
                break;

            case 'X': //58
                drawLetter(4, "10001 "
                            + "01010 "
                            + "00100 "
                            + "01010 "
                            + "10001n");
                break;

            case 'Y': //59
                drawLetter(4, "10001 "
                            + "01010 "
                            + "00100 "
                            + "00100 "
                            + "00100n");
                break;

            case 'Z': //5A
                drawLetter(4, "11111 "
                            + "00010 "
                            + "00100 "
                            + "01000 "
                            + "11111n");
                break;
            }
        }
    }
}

Both of the above can be found on my Github project.

My main question stems from the drawLetter method in the second class. Is there a better way to implement it than parsing a string that is meant to serve as a grid where the robot to place beepers?

Additionally, is there anything else I can do to improve the above code? Keep in mind that I am a beginner to Java.

\$\endgroup\$
1
\$\begingroup\$

Here are some comments from a somewhat rusty Java programmer, which from the get go would like to state that your code does look clean and good. Somewhat repetitive at times, but that is possibly hard to avoid given your base classes.

  • If the direction is known, and to be trusted, I would rather use a switch statement on currentDirection within the move() function. This would eliminate some calls to facingXxxxx() and would read a little easier
  • Feature: Can you trust currentDirection? – Within the faceXxxx() you set the new direction, but then in turnXxx() and leftTurns() you don't. If not the direction is available (i.e. from super), I would possibly implement a find direction method which spins the robot 4 times to determine the actual current direction. (But this could confusing if you see the robot turning on the screen :-) )

    Within getCurrentDirection() and printRobotInformation() you assume that currentDirection is correct. This could easily be wrong if some turn commands has been issued.

  • Implement the faceXxxx() in a general method? – Yet again, if you can trust the direction, I would make a method faceThisDirection(newDirection), and then use this from all of the faceXxxx() methods. This way you would reduce the code handling facing a given direction into one method, with convenience method available.

    If at any point the turning of robots changes, you could then change the one main facing method, and the other would magically be updated.

  • Reduce turns in coordinateChange() – If you see the robot spinning on the screen I would consider trying to reduce the spinning in this method. I.e. if it is facing south, and you want it to move 2 to the West, and 2 to the North, it would spin twice to achieve this with the current code.

    This could be avoided if you are aware of which direction you are turning, and then checking if the robot needs to move in this direction. In addition you could then also enforce that the robot faces the same way as it started. The current version could be considered as a bug/feature, as it might face a different way after changing the coordinate.

  • For error messages use System.err.println() & co – Instead of System.out you should use System.err for outputting error messages. This way the end user might handle them even better
  • In beeperToASCII() I would add some leading text – Currently it just report the number of beepers it has picked up, with no context. This could be hard to understand what it is when viewing in the console. Using something like "Picked up XX beepers" would be better.
  • Having both xCoordinate and getXCoordinate() public is redundant – If the variable is public, you don't really need the a getter method, you could just as easily use the actual variable. This of course applies to all of your public variables.

Lets move on to the LetterRobot implementation which also seems like a neat robot extension:

  • Feature: Unknown direction when starting drawLetter() – You don't know the direction the robot is facing when starting here, and as such the first move is kind of random.
  • Introduce a temporary variable to hold current operation within drawLetter() – I would prefer to have an extra variable to hold the letterGrid.charAt(numberIndex) character, as it would avoid doing this call multiple times and read a little easier.
  • Possibly remove n operand – I would consider always make the drawLetter() do the n operation at end of any string, thusly removing the need for it to be present.
  • Simplify the operation of drawLetter() – If you at the start turn the robot a given direction, i.e. faceEast() you could possibly simplify the remaining three direction, and move the setting of the maximumXCoordinate out of the if statements. This would further simplify the operation, whilst maintaining functionality.
  • Consider switching to an array representation for the letters – I do like the way you've solved this, but just thinking that if you had an array of the different rows of the letter you wouldn't need the distanceToTop and it could possibly be even more flexible for alternate patterns. You could possibly also then set the maximumXCoordinate at the start of the loop based on longest string in the array. Just a thought.
  • Within drawString() you silently ignore unknown characters – Any character not within the uppercase set of A through Z is silently ignored. At least I would consider printing an error message, and possibly to uppercase any strings passed to this method
  • Consider defining a font array – The drawString() is rather long due to the presence of all the definitions of the different character. This font array could be defined as constant outside of the method, allowing for a much simpler method of simply calling drawLetter() with an array matching the character to print, instead of the largish switch statement.

That was a lot of text, but do keep in mind that a lot of this are suggestions not explicit errors. In general it seems like your coding style is good, and that you've implemented good extensions to the robot library. And in the world of programming there is always room for other implementations, or other viewpoints. So keep up the good work, and don't be discouraged even though this got to be a somewhat lengthy review.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the review! I have some responses for the LetterRobot responses (hopefully that's OK). First off, I should mention that the rest of the printable ASCII characters will be added, but I didn't want to commit to the syntax I had and change over a hundred statements later if there were a problem, so I only wrote the uppercase letters. distanceToTop is needed because when I implement lower case letters, letters like a will start 2 above the original y-coordinate, not 4. The alternative, of course, is to hardcode a move(4) and have two rows of 0s before a row containing 1s. \$\endgroup\$ – Arcturus Feb 21 '16 at 22:56
  • \$\begingroup\$ @Eridan Or you could add some extra empty lines so that all characters have the same size, and start at the same place \$\endgroup\$ – holroy Feb 21 '16 at 22:58
  • \$\begingroup\$ Since some lowercase characters extend below the original line, that would mean making the strings for A and y respectively the following (n has been changed): 0110 1001 1111 1001 1001 0000 0000 and 000 000 101 101 111 001 111. Is that what you mean? \$\endgroup\$ – Arcturus Feb 21 '16 at 23:02
  • \$\begingroup\$ @Eridan Given that you always start at top left, you don't really need the the last two empty lined for the "A", just keep track of how many lines you've gone done. \$\endgroup\$ – holroy Feb 21 '16 at 23:04
  • \$\begingroup\$ Isn't that what I do with the parameter distanceToTop? That has the intent of positioning the robot at the top left of the specific letter, and then coordinateChange(maximumXCoordinate + 1, firstYCoordinate) positions the robot at the start of the next letter. \$\endgroup\$ – Arcturus Feb 22 '16 at 2:59

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.