Robot Block Command











up vote
2
down vote

favorite













Problem



We have a robot that can pickup blocks from a stash, move them
horizontally and lower them in place. There are 10 positions available
to lower blocks number 0 to 9. Each position can hold up to 15 blocks.



The robot understands the commands 'P', 'M' and 'L':




  • P: Pickup from the stash and move to position 0

  • M: Move to the Next Position

  • L: Lower the block


The robot is safe to operate and very forgiving:




  • There are always blocks in the stash (Pickup always gets a block)

  • If the robot already holds a block, Pickup will reset position to 0

  • The robot will not go beyond position 9, Trying to move further does nothing

  • Lowering the block on a pile of 15 does nothing

  • Lowering without a block does nothing

  • Robot ignores any command that is not 'P', 'M','L'


Implement a function that takes a String of commands for the robot.
The function should output String representing the number of blocks
(in hexadecimal) at each position after running all the commands



enter image description here






Solution



I have implemented it and it works fine and all but I feel like this is not the correct way.



Key errors I feel are how I determine Hexadecimal values from A to F. What is the correct way of doing this other than using a hashmap?



Also, processing of each command using switch-case feels so verbose as well.



Has anyone solved this problem before? What's the optimal way of doing this?



private static int stash = new int[10];
private static HashMap<Integer, String> map = new HashMap<Integer, String>();
public static void main(String args) {
map.put(10, "A");
map.put(11, "B");
map.put(12, "C");
map.put(13, "D");
map.put(14, "E");
map.put(15, "F");

String command = "PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMMMLPMLPMMLPMMMMMMMMMMLPMMMMMMMMMML";
executeCommand(command);
}

private static void executeCommand(String command) {
int block = 0;
int stashLocation = 0;
for (Character c : command.toCharArray()) {
switch (c) {
case 'P':
if (block == 0) {
block = 1;
} else {
stashLocation = 0;
}
break;
case 'M':
if (stashLocation < 9) {
stashLocation++;
}
break;
case 'L':
if (block != 0 && stash[stashLocation] < 15) {
stash[stashLocation] = stash[stashLocation] + 1;
block = 0;
stashLocation = 0;
}
break;
}
}
Arrays.stream(stash).forEach(val -> {
if (map.containsKey(val)) {
System.out.print(map.get(val));
} else {
System.out.print(val);
}
});
}









share|improve this question









New contributor




mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.




















  • P and L actions do not match the problem statement (P should always move arm to position 0, L should not change the arm position).
    – vnp
    yesterday















up vote
2
down vote

favorite













Problem



We have a robot that can pickup blocks from a stash, move them
horizontally and lower them in place. There are 10 positions available
to lower blocks number 0 to 9. Each position can hold up to 15 blocks.



The robot understands the commands 'P', 'M' and 'L':




  • P: Pickup from the stash and move to position 0

  • M: Move to the Next Position

  • L: Lower the block


The robot is safe to operate and very forgiving:




  • There are always blocks in the stash (Pickup always gets a block)

  • If the robot already holds a block, Pickup will reset position to 0

  • The robot will not go beyond position 9, Trying to move further does nothing

  • Lowering the block on a pile of 15 does nothing

  • Lowering without a block does nothing

  • Robot ignores any command that is not 'P', 'M','L'


Implement a function that takes a String of commands for the robot.
The function should output String representing the number of blocks
(in hexadecimal) at each position after running all the commands



enter image description here






Solution



I have implemented it and it works fine and all but I feel like this is not the correct way.



Key errors I feel are how I determine Hexadecimal values from A to F. What is the correct way of doing this other than using a hashmap?



Also, processing of each command using switch-case feels so verbose as well.



Has anyone solved this problem before? What's the optimal way of doing this?



private static int stash = new int[10];
private static HashMap<Integer, String> map = new HashMap<Integer, String>();
public static void main(String args) {
map.put(10, "A");
map.put(11, "B");
map.put(12, "C");
map.put(13, "D");
map.put(14, "E");
map.put(15, "F");

String command = "PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMMMLPMLPMMLPMMMMMMMMMMLPMMMMMMMMMML";
executeCommand(command);
}

private static void executeCommand(String command) {
int block = 0;
int stashLocation = 0;
for (Character c : command.toCharArray()) {
switch (c) {
case 'P':
if (block == 0) {
block = 1;
} else {
stashLocation = 0;
}
break;
case 'M':
if (stashLocation < 9) {
stashLocation++;
}
break;
case 'L':
if (block != 0 && stash[stashLocation] < 15) {
stash[stashLocation] = stash[stashLocation] + 1;
block = 0;
stashLocation = 0;
}
break;
}
}
Arrays.stream(stash).forEach(val -> {
if (map.containsKey(val)) {
System.out.print(map.get(val));
} else {
System.out.print(val);
}
});
}









share|improve this question









New contributor




mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.




















  • P and L actions do not match the problem statement (P should always move arm to position 0, L should not change the arm position).
    – vnp
    yesterday













up vote
2
down vote

favorite









up vote
2
down vote

favorite












Problem



We have a robot that can pickup blocks from a stash, move them
horizontally and lower them in place. There are 10 positions available
to lower blocks number 0 to 9. Each position can hold up to 15 blocks.



The robot understands the commands 'P', 'M' and 'L':




  • P: Pickup from the stash and move to position 0

  • M: Move to the Next Position

  • L: Lower the block


The robot is safe to operate and very forgiving:




  • There are always blocks in the stash (Pickup always gets a block)

  • If the robot already holds a block, Pickup will reset position to 0

  • The robot will not go beyond position 9, Trying to move further does nothing

  • Lowering the block on a pile of 15 does nothing

  • Lowering without a block does nothing

  • Robot ignores any command that is not 'P', 'M','L'


Implement a function that takes a String of commands for the robot.
The function should output String representing the number of blocks
(in hexadecimal) at each position after running all the commands



enter image description here






Solution



I have implemented it and it works fine and all but I feel like this is not the correct way.



Key errors I feel are how I determine Hexadecimal values from A to F. What is the correct way of doing this other than using a hashmap?



Also, processing of each command using switch-case feels so verbose as well.



Has anyone solved this problem before? What's the optimal way of doing this?



private static int stash = new int[10];
private static HashMap<Integer, String> map = new HashMap<Integer, String>();
public static void main(String args) {
map.put(10, "A");
map.put(11, "B");
map.put(12, "C");
map.put(13, "D");
map.put(14, "E");
map.put(15, "F");

String command = "PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMMMLPMLPMMLPMMMMMMMMMMLPMMMMMMMMMML";
executeCommand(command);
}

private static void executeCommand(String command) {
int block = 0;
int stashLocation = 0;
for (Character c : command.toCharArray()) {
switch (c) {
case 'P':
if (block == 0) {
block = 1;
} else {
stashLocation = 0;
}
break;
case 'M':
if (stashLocation < 9) {
stashLocation++;
}
break;
case 'L':
if (block != 0 && stash[stashLocation] < 15) {
stash[stashLocation] = stash[stashLocation] + 1;
block = 0;
stashLocation = 0;
}
break;
}
}
Arrays.stream(stash).forEach(val -> {
if (map.containsKey(val)) {
System.out.print(map.get(val));
} else {
System.out.print(val);
}
});
}









share|improve this question









New contributor




mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.












Problem



We have a robot that can pickup blocks from a stash, move them
horizontally and lower them in place. There are 10 positions available
to lower blocks number 0 to 9. Each position can hold up to 15 blocks.



The robot understands the commands 'P', 'M' and 'L':




  • P: Pickup from the stash and move to position 0

  • M: Move to the Next Position

  • L: Lower the block


The robot is safe to operate and very forgiving:




  • There are always blocks in the stash (Pickup always gets a block)

  • If the robot already holds a block, Pickup will reset position to 0

  • The robot will not go beyond position 9, Trying to move further does nothing

  • Lowering the block on a pile of 15 does nothing

  • Lowering without a block does nothing

  • Robot ignores any command that is not 'P', 'M','L'


Implement a function that takes a String of commands for the robot.
The function should output String representing the number of blocks
(in hexadecimal) at each position after running all the commands



enter image description here






Solution



I have implemented it and it works fine and all but I feel like this is not the correct way.



Key errors I feel are how I determine Hexadecimal values from A to F. What is the correct way of doing this other than using a hashmap?



Also, processing of each command using switch-case feels so verbose as well.



Has anyone solved this problem before? What's the optimal way of doing this?



private static int stash = new int[10];
private static HashMap<Integer, String> map = new HashMap<Integer, String>();
public static void main(String args) {
map.put(10, "A");
map.put(11, "B");
map.put(12, "C");
map.put(13, "D");
map.put(14, "E");
map.put(15, "F");

String command = "PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMMMLPMLPMMLPMMMMMMMMMMLPMMMMMMMMMML";
executeCommand(command);
}

private static void executeCommand(String command) {
int block = 0;
int stashLocation = 0;
for (Character c : command.toCharArray()) {
switch (c) {
case 'P':
if (block == 0) {
block = 1;
} else {
stashLocation = 0;
}
break;
case 'M':
if (stashLocation < 9) {
stashLocation++;
}
break;
case 'L':
if (block != 0 && stash[stashLocation] < 15) {
stash[stashLocation] = stash[stashLocation] + 1;
block = 0;
stashLocation = 0;
}
break;
}
}
Arrays.stream(stash).forEach(val -> {
if (map.containsKey(val)) {
System.out.print(map.get(val));
} else {
System.out.print(val);
}
});
}






java algorithm






share|improve this question









New contributor




mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.











share|improve this question









New contributor




mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









share|improve this question




share|improve this question








edited 23 hours ago









Jamal

30.2k11115226




30.2k11115226






New contributor




mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









asked yesterday









mel3kings

1113




1113




New contributor




mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.





New contributor





mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






mel3kings is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.












  • P and L actions do not match the problem statement (P should always move arm to position 0, L should not change the arm position).
    – vnp
    yesterday


















  • P and L actions do not match the problem statement (P should always move arm to position 0, L should not change the arm position).
    – vnp
    yesterday
















P and L actions do not match the problem statement (P should always move arm to position 0, L should not change the arm position).
– vnp
yesterday




P and L actions do not match the problem statement (P should always move arm to position 0, L should not change the arm position).
– vnp
yesterday










1 Answer
1






active

oldest

votes

















up vote
0
down vote













Your code doesn't really take advantage of the object oriented features of Java. You have everything in one file with one object manipulating the state of several things: The robotic arm, the stash, and the piles. This makes the code tightly coupled and hard to change.
Generally, your variable naming is clear and follows Java best practices: block, stashLocation, etc. Personally I'd use a boolean for the block, not an int, as your robot either has or doesn't have a block. That'd also clean up the if clauses:



boolean hasBlock = false;
// other code here
if (hasBlock) {
doSomethingWithBlock();
}


Regarding your specific questions:



Hexadecimal values



Use Java's print formatting functions String.format and it's likes:
String hex = String.format("%02X", 15) will assign the value 0xF to the variable hex. Read up on String.format and the format specifiers



Command processing



For this case, I think it's fine to use a switch case to act on the commands. Somehow, you have to identify each single command char, and a switch is a good way here. What I would do, though, is break out the actual commands into separate functions, and only call them from the case statement:



for (char cmd : command.toCharArray()) {
switch (cmd) {
case 'P':
pickup();
break;
case 'M':
move();
break;
case 'L':
lower();
break;
default:
System.out.format("Unknown command %c", cmd);
}
}

void pickup() { /* function definition here */ }
void move() { /* function definition here */ }
void lower() { /* function definition here */ }


Of course, one could get fancy here and map the command chars to a lambda or something, but that might be overkill. A small improvement could be to use an enum for the commands, and switch on that.



My implementation



This was a fun task, and here is my implementation. I split the Robot and the Piles into own classes, coordinated by the Main class. I thought about separating the Stash also, but since it had next to no business logic, I left in in the Robot (just setting hasBlock to true in pickup). By using objects, the printing of status can be "automated" by using the toString() method, as I've done below.



Piles.java



public class Piles {

public Piles(int width, int height) {
this.width = width - 1; // - 1 because of zreo indexing
this.height = height;
piles = new int[width];
}

public boolean drop(int position) {
boolean positionValid = position <= (this.width - 1);
boolean pileAvailable = this.piles[position] < this.height;

if (positionValid && pileAvailable) {
piles[position]++;
}

return positionValid && pileAvailable;
}

@Override
public String toString() {
StringBuffer res = new StringBuffer();

for (int i = 0; i < piles.length; i++) {
res.append(String.format("%d: %02x ", i, piles[i]));
}
return res.toString();
}

public int getWidth() {
return this.width;
}

public int getHeight() {
return this.height;
}

/* PRIVATE */
private final int width;
private final int height;

private final int piles;

}


Robot.java



public class Robot {

public Robot(Piles piles) {
this.maxWidth = piles.getWidth();
this.piles = piles;

}

public void execute(String command) {
for (char cmd : command.toCharArray()) {
switch (cmd) {
case 'P':
pickup();
break;
case 'M':
move();
break;
case 'L':
lower();
break;
default:
System.out.format("Unknown command %c", cmd);
}
}
System.out.println(piles);
}

@Override
public String toString() {
return String.format("Robot is at position %d %s a block", position, hasBlock ? "with" : "without");
}

/* PRIVATE */

private final Piles piles;
private final int maxWidth;
private int position = 0;
private boolean hasBlock = false;

/* Pick up a block from the stash.
* The rules are:
* - the stash is never empty
* - after pickup, move to position 0
* - if the robot already holds a block, reset position to 0
*/
private void pickup() {
hasBlock = true;
position = 0;
}


/* Advance the robot one step
* The rules are:
* - the robot will not go beyond maxWidth, trying to move further does nothing.
*/
private void move() {
position = Math.min(++position, maxWidth);
}


/* Lower a block to the pile at the current location.
* The rules are:
* - lowering the block on a pile of 15 does nothing
* - lowering without a block does nothing
*/
private void lower() {
if (piles.drop(position)) {
hasBlock = false;
}
}
}


Main.java



public class Main {

public static void main(String args) {

Robot robot = new Robot(new Piles(10, 15));
System.out.println(robot);

robot.execute("PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPML");
robot.execute("PLPLPMMMMML");

}
}





share|improve this answer





















    Your Answer





    StackExchange.ifUsing("editor", function () {
    return StackExchange.using("mathjaxEditing", function () {
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    });
    });
    }, "mathjax-editing");

    StackExchange.ifUsing("editor", function () {
    StackExchange.using("externalEditor", function () {
    StackExchange.using("snippets", function () {
    StackExchange.snippets.init();
    });
    });
    }, "code-snippets");

    StackExchange.ready(function() {
    var channelOptions = {
    tags: "".split(" "),
    id: "196"
    };
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function() {
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled) {
    StackExchange.using("snippets", function() {
    createEditor();
    });
    }
    else {
    createEditor();
    }
    });

    function createEditor() {
    StackExchange.prepareEditor({
    heartbeatType: 'answer',
    convertImagesToLinks: false,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    imageUploader: {
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    },
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    });


    }
    });






    mel3kings is a new contributor. Be nice, and check out our Code of Conduct.










    draft saved

    draft discarded


















    StackExchange.ready(
    function () {
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209155%2frobot-block-command%23new-answer', 'question_page');
    }
    );

    Post as a guest















    Required, but never shown

























    1 Answer
    1






    active

    oldest

    votes








    1 Answer
    1






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes








    up vote
    0
    down vote













    Your code doesn't really take advantage of the object oriented features of Java. You have everything in one file with one object manipulating the state of several things: The robotic arm, the stash, and the piles. This makes the code tightly coupled and hard to change.
    Generally, your variable naming is clear and follows Java best practices: block, stashLocation, etc. Personally I'd use a boolean for the block, not an int, as your robot either has or doesn't have a block. That'd also clean up the if clauses:



    boolean hasBlock = false;
    // other code here
    if (hasBlock) {
    doSomethingWithBlock();
    }


    Regarding your specific questions:



    Hexadecimal values



    Use Java's print formatting functions String.format and it's likes:
    String hex = String.format("%02X", 15) will assign the value 0xF to the variable hex. Read up on String.format and the format specifiers



    Command processing



    For this case, I think it's fine to use a switch case to act on the commands. Somehow, you have to identify each single command char, and a switch is a good way here. What I would do, though, is break out the actual commands into separate functions, and only call them from the case statement:



    for (char cmd : command.toCharArray()) {
    switch (cmd) {
    case 'P':
    pickup();
    break;
    case 'M':
    move();
    break;
    case 'L':
    lower();
    break;
    default:
    System.out.format("Unknown command %c", cmd);
    }
    }

    void pickup() { /* function definition here */ }
    void move() { /* function definition here */ }
    void lower() { /* function definition here */ }


    Of course, one could get fancy here and map the command chars to a lambda or something, but that might be overkill. A small improvement could be to use an enum for the commands, and switch on that.



    My implementation



    This was a fun task, and here is my implementation. I split the Robot and the Piles into own classes, coordinated by the Main class. I thought about separating the Stash also, but since it had next to no business logic, I left in in the Robot (just setting hasBlock to true in pickup). By using objects, the printing of status can be "automated" by using the toString() method, as I've done below.



    Piles.java



    public class Piles {

    public Piles(int width, int height) {
    this.width = width - 1; // - 1 because of zreo indexing
    this.height = height;
    piles = new int[width];
    }

    public boolean drop(int position) {
    boolean positionValid = position <= (this.width - 1);
    boolean pileAvailable = this.piles[position] < this.height;

    if (positionValid && pileAvailable) {
    piles[position]++;
    }

    return positionValid && pileAvailable;
    }

    @Override
    public String toString() {
    StringBuffer res = new StringBuffer();

    for (int i = 0; i < piles.length; i++) {
    res.append(String.format("%d: %02x ", i, piles[i]));
    }
    return res.toString();
    }

    public int getWidth() {
    return this.width;
    }

    public int getHeight() {
    return this.height;
    }

    /* PRIVATE */
    private final int width;
    private final int height;

    private final int piles;

    }


    Robot.java



    public class Robot {

    public Robot(Piles piles) {
    this.maxWidth = piles.getWidth();
    this.piles = piles;

    }

    public void execute(String command) {
    for (char cmd : command.toCharArray()) {
    switch (cmd) {
    case 'P':
    pickup();
    break;
    case 'M':
    move();
    break;
    case 'L':
    lower();
    break;
    default:
    System.out.format("Unknown command %c", cmd);
    }
    }
    System.out.println(piles);
    }

    @Override
    public String toString() {
    return String.format("Robot is at position %d %s a block", position, hasBlock ? "with" : "without");
    }

    /* PRIVATE */

    private final Piles piles;
    private final int maxWidth;
    private int position = 0;
    private boolean hasBlock = false;

    /* Pick up a block from the stash.
    * The rules are:
    * - the stash is never empty
    * - after pickup, move to position 0
    * - if the robot already holds a block, reset position to 0
    */
    private void pickup() {
    hasBlock = true;
    position = 0;
    }


    /* Advance the robot one step
    * The rules are:
    * - the robot will not go beyond maxWidth, trying to move further does nothing.
    */
    private void move() {
    position = Math.min(++position, maxWidth);
    }


    /* Lower a block to the pile at the current location.
    * The rules are:
    * - lowering the block on a pile of 15 does nothing
    * - lowering without a block does nothing
    */
    private void lower() {
    if (piles.drop(position)) {
    hasBlock = false;
    }
    }
    }


    Main.java



    public class Main {

    public static void main(String args) {

    Robot robot = new Robot(new Piles(10, 15));
    System.out.println(robot);

    robot.execute("PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPML");
    robot.execute("PLPLPMMMMML");

    }
    }





    share|improve this answer

























      up vote
      0
      down vote













      Your code doesn't really take advantage of the object oriented features of Java. You have everything in one file with one object manipulating the state of several things: The robotic arm, the stash, and the piles. This makes the code tightly coupled and hard to change.
      Generally, your variable naming is clear and follows Java best practices: block, stashLocation, etc. Personally I'd use a boolean for the block, not an int, as your robot either has or doesn't have a block. That'd also clean up the if clauses:



      boolean hasBlock = false;
      // other code here
      if (hasBlock) {
      doSomethingWithBlock();
      }


      Regarding your specific questions:



      Hexadecimal values



      Use Java's print formatting functions String.format and it's likes:
      String hex = String.format("%02X", 15) will assign the value 0xF to the variable hex. Read up on String.format and the format specifiers



      Command processing



      For this case, I think it's fine to use a switch case to act on the commands. Somehow, you have to identify each single command char, and a switch is a good way here. What I would do, though, is break out the actual commands into separate functions, and only call them from the case statement:



      for (char cmd : command.toCharArray()) {
      switch (cmd) {
      case 'P':
      pickup();
      break;
      case 'M':
      move();
      break;
      case 'L':
      lower();
      break;
      default:
      System.out.format("Unknown command %c", cmd);
      }
      }

      void pickup() { /* function definition here */ }
      void move() { /* function definition here */ }
      void lower() { /* function definition here */ }


      Of course, one could get fancy here and map the command chars to a lambda or something, but that might be overkill. A small improvement could be to use an enum for the commands, and switch on that.



      My implementation



      This was a fun task, and here is my implementation. I split the Robot and the Piles into own classes, coordinated by the Main class. I thought about separating the Stash also, but since it had next to no business logic, I left in in the Robot (just setting hasBlock to true in pickup). By using objects, the printing of status can be "automated" by using the toString() method, as I've done below.



      Piles.java



      public class Piles {

      public Piles(int width, int height) {
      this.width = width - 1; // - 1 because of zreo indexing
      this.height = height;
      piles = new int[width];
      }

      public boolean drop(int position) {
      boolean positionValid = position <= (this.width - 1);
      boolean pileAvailable = this.piles[position] < this.height;

      if (positionValid && pileAvailable) {
      piles[position]++;
      }

      return positionValid && pileAvailable;
      }

      @Override
      public String toString() {
      StringBuffer res = new StringBuffer();

      for (int i = 0; i < piles.length; i++) {
      res.append(String.format("%d: %02x ", i, piles[i]));
      }
      return res.toString();
      }

      public int getWidth() {
      return this.width;
      }

      public int getHeight() {
      return this.height;
      }

      /* PRIVATE */
      private final int width;
      private final int height;

      private final int piles;

      }


      Robot.java



      public class Robot {

      public Robot(Piles piles) {
      this.maxWidth = piles.getWidth();
      this.piles = piles;

      }

      public void execute(String command) {
      for (char cmd : command.toCharArray()) {
      switch (cmd) {
      case 'P':
      pickup();
      break;
      case 'M':
      move();
      break;
      case 'L':
      lower();
      break;
      default:
      System.out.format("Unknown command %c", cmd);
      }
      }
      System.out.println(piles);
      }

      @Override
      public String toString() {
      return String.format("Robot is at position %d %s a block", position, hasBlock ? "with" : "without");
      }

      /* PRIVATE */

      private final Piles piles;
      private final int maxWidth;
      private int position = 0;
      private boolean hasBlock = false;

      /* Pick up a block from the stash.
      * The rules are:
      * - the stash is never empty
      * - after pickup, move to position 0
      * - if the robot already holds a block, reset position to 0
      */
      private void pickup() {
      hasBlock = true;
      position = 0;
      }


      /* Advance the robot one step
      * The rules are:
      * - the robot will not go beyond maxWidth, trying to move further does nothing.
      */
      private void move() {
      position = Math.min(++position, maxWidth);
      }


      /* Lower a block to the pile at the current location.
      * The rules are:
      * - lowering the block on a pile of 15 does nothing
      * - lowering without a block does nothing
      */
      private void lower() {
      if (piles.drop(position)) {
      hasBlock = false;
      }
      }
      }


      Main.java



      public class Main {

      public static void main(String args) {

      Robot robot = new Robot(new Piles(10, 15));
      System.out.println(robot);

      robot.execute("PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPML");
      robot.execute("PLPLPMMMMML");

      }
      }





      share|improve this answer























        up vote
        0
        down vote










        up vote
        0
        down vote









        Your code doesn't really take advantage of the object oriented features of Java. You have everything in one file with one object manipulating the state of several things: The robotic arm, the stash, and the piles. This makes the code tightly coupled and hard to change.
        Generally, your variable naming is clear and follows Java best practices: block, stashLocation, etc. Personally I'd use a boolean for the block, not an int, as your robot either has or doesn't have a block. That'd also clean up the if clauses:



        boolean hasBlock = false;
        // other code here
        if (hasBlock) {
        doSomethingWithBlock();
        }


        Regarding your specific questions:



        Hexadecimal values



        Use Java's print formatting functions String.format and it's likes:
        String hex = String.format("%02X", 15) will assign the value 0xF to the variable hex. Read up on String.format and the format specifiers



        Command processing



        For this case, I think it's fine to use a switch case to act on the commands. Somehow, you have to identify each single command char, and a switch is a good way here. What I would do, though, is break out the actual commands into separate functions, and only call them from the case statement:



        for (char cmd : command.toCharArray()) {
        switch (cmd) {
        case 'P':
        pickup();
        break;
        case 'M':
        move();
        break;
        case 'L':
        lower();
        break;
        default:
        System.out.format("Unknown command %c", cmd);
        }
        }

        void pickup() { /* function definition here */ }
        void move() { /* function definition here */ }
        void lower() { /* function definition here */ }


        Of course, one could get fancy here and map the command chars to a lambda or something, but that might be overkill. A small improvement could be to use an enum for the commands, and switch on that.



        My implementation



        This was a fun task, and here is my implementation. I split the Robot and the Piles into own classes, coordinated by the Main class. I thought about separating the Stash also, but since it had next to no business logic, I left in in the Robot (just setting hasBlock to true in pickup). By using objects, the printing of status can be "automated" by using the toString() method, as I've done below.



        Piles.java



        public class Piles {

        public Piles(int width, int height) {
        this.width = width - 1; // - 1 because of zreo indexing
        this.height = height;
        piles = new int[width];
        }

        public boolean drop(int position) {
        boolean positionValid = position <= (this.width - 1);
        boolean pileAvailable = this.piles[position] < this.height;

        if (positionValid && pileAvailable) {
        piles[position]++;
        }

        return positionValid && pileAvailable;
        }

        @Override
        public String toString() {
        StringBuffer res = new StringBuffer();

        for (int i = 0; i < piles.length; i++) {
        res.append(String.format("%d: %02x ", i, piles[i]));
        }
        return res.toString();
        }

        public int getWidth() {
        return this.width;
        }

        public int getHeight() {
        return this.height;
        }

        /* PRIVATE */
        private final int width;
        private final int height;

        private final int piles;

        }


        Robot.java



        public class Robot {

        public Robot(Piles piles) {
        this.maxWidth = piles.getWidth();
        this.piles = piles;

        }

        public void execute(String command) {
        for (char cmd : command.toCharArray()) {
        switch (cmd) {
        case 'P':
        pickup();
        break;
        case 'M':
        move();
        break;
        case 'L':
        lower();
        break;
        default:
        System.out.format("Unknown command %c", cmd);
        }
        }
        System.out.println(piles);
        }

        @Override
        public String toString() {
        return String.format("Robot is at position %d %s a block", position, hasBlock ? "with" : "without");
        }

        /* PRIVATE */

        private final Piles piles;
        private final int maxWidth;
        private int position = 0;
        private boolean hasBlock = false;

        /* Pick up a block from the stash.
        * The rules are:
        * - the stash is never empty
        * - after pickup, move to position 0
        * - if the robot already holds a block, reset position to 0
        */
        private void pickup() {
        hasBlock = true;
        position = 0;
        }


        /* Advance the robot one step
        * The rules are:
        * - the robot will not go beyond maxWidth, trying to move further does nothing.
        */
        private void move() {
        position = Math.min(++position, maxWidth);
        }


        /* Lower a block to the pile at the current location.
        * The rules are:
        * - lowering the block on a pile of 15 does nothing
        * - lowering without a block does nothing
        */
        private void lower() {
        if (piles.drop(position)) {
        hasBlock = false;
        }
        }
        }


        Main.java



        public class Main {

        public static void main(String args) {

        Robot robot = new Robot(new Piles(10, 15));
        System.out.println(robot);

        robot.execute("PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPML");
        robot.execute("PLPLPMMMMML");

        }
        }





        share|improve this answer












        Your code doesn't really take advantage of the object oriented features of Java. You have everything in one file with one object manipulating the state of several things: The robotic arm, the stash, and the piles. This makes the code tightly coupled and hard to change.
        Generally, your variable naming is clear and follows Java best practices: block, stashLocation, etc. Personally I'd use a boolean for the block, not an int, as your robot either has or doesn't have a block. That'd also clean up the if clauses:



        boolean hasBlock = false;
        // other code here
        if (hasBlock) {
        doSomethingWithBlock();
        }


        Regarding your specific questions:



        Hexadecimal values



        Use Java's print formatting functions String.format and it's likes:
        String hex = String.format("%02X", 15) will assign the value 0xF to the variable hex. Read up on String.format and the format specifiers



        Command processing



        For this case, I think it's fine to use a switch case to act on the commands. Somehow, you have to identify each single command char, and a switch is a good way here. What I would do, though, is break out the actual commands into separate functions, and only call them from the case statement:



        for (char cmd : command.toCharArray()) {
        switch (cmd) {
        case 'P':
        pickup();
        break;
        case 'M':
        move();
        break;
        case 'L':
        lower();
        break;
        default:
        System.out.format("Unknown command %c", cmd);
        }
        }

        void pickup() { /* function definition here */ }
        void move() { /* function definition here */ }
        void lower() { /* function definition here */ }


        Of course, one could get fancy here and map the command chars to a lambda or something, but that might be overkill. A small improvement could be to use an enum for the commands, and switch on that.



        My implementation



        This was a fun task, and here is my implementation. I split the Robot and the Piles into own classes, coordinated by the Main class. I thought about separating the Stash also, but since it had next to no business logic, I left in in the Robot (just setting hasBlock to true in pickup). By using objects, the printing of status can be "automated" by using the toString() method, as I've done below.



        Piles.java



        public class Piles {

        public Piles(int width, int height) {
        this.width = width - 1; // - 1 because of zreo indexing
        this.height = height;
        piles = new int[width];
        }

        public boolean drop(int position) {
        boolean positionValid = position <= (this.width - 1);
        boolean pileAvailable = this.piles[position] < this.height;

        if (positionValid && pileAvailable) {
        piles[position]++;
        }

        return positionValid && pileAvailable;
        }

        @Override
        public String toString() {
        StringBuffer res = new StringBuffer();

        for (int i = 0; i < piles.length; i++) {
        res.append(String.format("%d: %02x ", i, piles[i]));
        }
        return res.toString();
        }

        public int getWidth() {
        return this.width;
        }

        public int getHeight() {
        return this.height;
        }

        /* PRIVATE */
        private final int width;
        private final int height;

        private final int piles;

        }


        Robot.java



        public class Robot {

        public Robot(Piles piles) {
        this.maxWidth = piles.getWidth();
        this.piles = piles;

        }

        public void execute(String command) {
        for (char cmd : command.toCharArray()) {
        switch (cmd) {
        case 'P':
        pickup();
        break;
        case 'M':
        move();
        break;
        case 'L':
        lower();
        break;
        default:
        System.out.format("Unknown command %c", cmd);
        }
        }
        System.out.println(piles);
        }

        @Override
        public String toString() {
        return String.format("Robot is at position %d %s a block", position, hasBlock ? "with" : "without");
        }

        /* PRIVATE */

        private final Piles piles;
        private final int maxWidth;
        private int position = 0;
        private boolean hasBlock = false;

        /* Pick up a block from the stash.
        * The rules are:
        * - the stash is never empty
        * - after pickup, move to position 0
        * - if the robot already holds a block, reset position to 0
        */
        private void pickup() {
        hasBlock = true;
        position = 0;
        }


        /* Advance the robot one step
        * The rules are:
        * - the robot will not go beyond maxWidth, trying to move further does nothing.
        */
        private void move() {
        position = Math.min(++position, maxWidth);
        }


        /* Lower a block to the pile at the current location.
        * The rules are:
        * - lowering the block on a pile of 15 does nothing
        * - lowering without a block does nothing
        */
        private void lower() {
        if (piles.drop(position)) {
        hasBlock = false;
        }
        }
        }


        Main.java



        public class Main {

        public static void main(String args) {

        Robot robot = new Robot(new Piles(10, 15));
        System.out.println(robot);

        robot.execute("PMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPMLPML");
        robot.execute("PLPLPMMMMML");

        }
        }






        share|improve this answer












        share|improve this answer



        share|improve this answer










        answered 15 hours ago









        TomG

        33116




        33116






















            mel3kings is a new contributor. Be nice, and check out our Code of Conduct.










            draft saved

            draft discarded


















            mel3kings is a new contributor. Be nice, and check out our Code of Conduct.













            mel3kings is a new contributor. Be nice, and check out our Code of Conduct.












            mel3kings is a new contributor. Be nice, and check out our Code of Conduct.
















            Thanks for contributing an answer to Code Review Stack Exchange!


            • Please be sure to answer the question. Provide details and share your research!

            But avoid



            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.


            Use MathJax to format equations. MathJax reference.


            To learn more, see our tips on writing great answers.





            Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


            Please pay close attention to the following guidance:


            • Please be sure to answer the question. Provide details and share your research!

            But avoid



            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.


            To learn more, see our tips on writing great answers.




            draft saved


            draft discarded














            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209155%2frobot-block-command%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown





















































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown

































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown







            Popular posts from this blog

            Mont Emei

            Province de Neuquén

            Journaliste