vendredi 27 décembre 2019

How could we refactor two related logics which are part of the same algorithm, into two separate methods‽

I am doing the following programming exercise: Traffic Lights - one car. Its statement is:

Overview

A character string represents a city road.

Cars travel on the road obeying the traffic lights..

Legend:

. = Road
C = Car
G = GREEN traffic light
O = ORANGE traffic light
R = RED traffic light

Something like this: C...R............G......

Rules Simulation

At each iteration:

the lights change, according to the traffic light rules... then
the car moves, obeying the car rules

Traffic Light Rules

Traffic lights change colour as follows:

GREEN for 5 time units... then
ORANGE for 1 time unit... then
RED for 5 time units....
... and repeat the cycle

Car Rules

Cars travel left to right on the road, moving 1 character position per time unit

Cars can move freely until they come to a traffic light. Then:
    if the light is GREEN they can move forward (temporarily occupying the same cell as the light)
    if the light is ORANGE then they must stop (if they have already entered the intersection they can continue through)
    if the light is RED the car must stop until the light turns GREEN again

Kata Task

Given the initial state of the road, return the states for all iterations of the simiulation. Input

road = the road array
n = how many time units to simulate (n >= 0)

Output

An array containing the road states at every iteration (including the initial state)
    Note: If a car occupies the same position as a traffic light then show only the car

Notes

There is only one car
For the initial road state
    the car is always at the first character position
    traffic lights are either GREEN or RED, and are at the beginning of their countdown cycles
There are no reaction delays - when the lights change the car drivers will react immediately!
If the car goes off the end of the road it just disappears from view
There will always be some road between adjacent traffic lights

Example

Run simulation for 10 time units

Input

road = "C...R............G......"
n = 10

Result

    [   "C...R............G......", // 0 initial state as passed  
    ".C..R............G......", // 1
   "..C.R............G......", // 2  
    "...CR............G......", // 3
   "...CR............G......", // 4  
    "....C............O......", // 5 show the car, not the light  
    "....GC...........R......", // 6
   "....G.C..........R......", // 7  
    "....G..C.........R......", // 8
   "....G...C........R......", // 9  
    "....O....C.......R......"  // 10 ]

First I have written the following code:

public class Dinglemouse {
  public static String[] trafficLights/*🚦🚦*/(String road, int n) {
    System.out.println("\n\nroad: "+road);
    System.out.println("\nn: "+n);
    String[] items = road.split("");
    int semaphoreTimeUnits = 0;
    boolean carHasMovedThisIteration = false;
    String hiddenSemaphore = "";
    String[] iterations = new String[n+1]; iterations[0] = road;

    for(int i = 0; i < n; i++, semaphoreTimeUnits++){
      //System.out.println("i: "+i);
      for(int j = 0; j < items.length - 2; j++){
        //System.out.println("j: "+j);
        //System.out.println("semaphoreTimeUnits: "+semaphoreTimeUnits);
        if(items[j].equals("C") && !carHasMovedThisIteration && !items[j+1].equals("R")){
          if(hiddenSemaphore.equals("")){
            items[j] = ".";
          }else{
            if(hiddenSemaphore.equals("O")){
              if(road.charAt(j) == 'R'){
                items[j] = "G";
              }else if(road.charAt(j) == 'G'){
                items[j] = "R";
              }else{
                items[j] = hiddenSemaphore;
              }
            }
            hiddenSemaphore = "";
          }
          if(!items[j+1].equals(".")){
            hiddenSemaphore = items[j+1];
          }
        items[j+1] = "C";
        carHasMovedThisIteration = true;
        }
        if(items[j+2].equals("R") && road.charAt(j+2) == 'R' && semaphoreTimeUnits >= 4
        || items[j+2].equals("G") && road.charAt(j+2) == 'G' && semaphoreTimeUnits >= 4){
          items[j+2] = "O";    
        }
        if(items[j+2].equals("O") && semaphoreTimeUnits >= 5){
          if(road.charAt(j+2) == 'R'){
            items[j+2] = "G";    
          }else if(road.charAt(j+2) == 'G'){
            items[j+2] = "R";  
          }
        }
        if((items[j+2].equals("G") || items[j+2].equals("R")) && semaphoreTimeUnits >= 10){
          items[j+2] = String.valueOf(road.charAt(j+2));    
          semaphoreTimeUnits = 0;
        }
        //System.out.println("items: "+String.join("", items));
      }
      iterations[i+1] = String.join("", items);
      System.out.println("items: "+String.join("", items));
      carHasMovedThisIteration = false;
    }
    return iterations;
  }
}

And the test (extracted from the exercise)

import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;


public class ExampleTests {

  private static void doTest(String initial, String[] expected, int n) {
    String got[] = Dinglemouse.trafficLights(initial, n);
    assertEquals("Result array length", expected.length, got.length);

    int errIdx = -1;
    for (int i = 0; i < got.length; i++) {
      if (!expected[i].equals(got[i])) { errIdx = i; break; }
    }

    System.out.println("Expected:");
    for (int i = 0; i < expected.length; i++) {
      System.out.println(String.format("%03d %s", i, Common.display(expected[i])));
    }

    if (errIdx >= 0) {
      System.out.println("\nYour result:");
      for (int i = 0; i < got.length; i++) {
        System.out.println(String.format("%03d %s", i, Common.display(got[i])));
      }
      fail(String.format("A difference was detected at index %d", errIdx));
    }
  }

  @Test
  public void example() {
    int n = 10;
    String sim[] = {
      "C...R............G......",  // 0
      ".C..R............G......",  // 1
      "..C.R............G......",  // 2
      "...CR............G......",  // 3
      "...CR............G......",  // 4
      "....C............O......",  // 5
      "....GC...........R......",  // 6
      "....G.C..........R......",  // 7
      "....G..C.........R......",  // 8
      "....G...C........R......",  // 9
      "....O....C.......R......"   // 10
    };
    doTest(sim[0], sim, n);
  }

}

Expected is:

Expected:
000 C...R............G......
001 .C..R............G......
002 ..C.R............G......
003 ...CR............G......
004 ...CR............G......
005 ....C............O......
006 ....GC...........R......
007 ....G.C..........R......
008 ....G..C.........R......
009 ....G...C........R......
010 ....O....C.......R......

The code output is:

Your result:

000 C...R............G......
001 .C..R............G......
002 ..C.R............G......
003 ...CR............G......
004 ...CR............G......
005 ....C............O......
006 ....GC...........R......
007 ....G.C..........R......
008 ....G..C.........R......
009 ....G...C........R......
010 ....G....C.......R......

So then as you will notice:

A difference was detected at index 10

Because of it expects an O instead of the G, at index 10.

1) How do we achieve that the car when is at an orange traffic light, the next 4 iterations it shows a green light, instead of the next 5 ones, as is being printed now.

2) How could we refactor, simplify the code, to separate car movement logic, and traffic ligths logic.

I have tried to extract car movement logic as:

public class Dinglemouse {
  public static String[] trafficLights/*🚦🚦*/(String road, int n) {
    System.out.println("\n\nroad: "+road);
    System.out.println("\nn: "+n);
    String[] items = road.split("");
    int semaphoreTimeUnits = 0;
    boolean carHasMovedThisIteration = false;
    String hiddenSemaphore = "";
    String[] iterations = new String[n+1]; iterations[0] = road;

    for(int i = 0; i < n; i++, semaphoreTimeUnits++){
      //System.out.println("i: "+i);
      for(int j = 0; j < items.length - 2; j++){
        //System.out.println("j: "+j);
        //System.out.println("semaphoreTimeUnits: "+semaphoreTimeUnits);

        simulateCarMovement(j, items, carHasMovedThisIteration, hiddenSemaphore, road);

        if(items[j+2].equals("R") && road.charAt(j+2) == 'R' && semaphoreTimeUnits >= 4
        || items[j+2].equals("G") && road.charAt(j+2) == 'G' && semaphoreTimeUnits >= 4){
          items[j+2] = "O";    
        }
        if(items[j+2].equals("O") && semaphoreTimeUnits >= 5){
          if(road.charAt(j+2) == 'R'){
            items[j+2] = "G";    
          }else if(road.charAt(j+2) == 'G'){
            items[j+2] = "R";  
          }
        }
        if((items[j+2].equals("G") || items[j+2].equals("R")) && semaphoreTimeUnits >= 10){
          items[j+2] = String.valueOf(road.charAt(j+2));    
          semaphoreTimeUnits = 0;
        }
        //System.out.println("items: "+String.join("", items));
      }
      iterations[i+1] = String.join("", items);
      System.out.println("items: "+String.join("", items));
      carHasMovedThisIteration = false;
    }
    return iterations;
  }


  static void simulateCarMovement(int j, String[] items, boolean carHasMovedThisIteration, String hiddenSemaphore, String road){
    if(items[j].equals("C") && !carHasMovedThisIteration && !items[j+1].equals("R")){
      if(hiddenSemaphore.equals("")){
        items[j] = ".";
      }else{
        if(hiddenSemaphore.equals("O")){
          if(road.charAt(j) == 'R'){
            items[j] = "G";
          }else if(road.charAt(j) == 'G'){
            items[j] = "R";
          }else{
            items[j] = hiddenSemaphore;
          }
        }
        hiddenSemaphore = "";
      }
      if(!items[j+1].equals(".")){
        hiddenSemaphore = items[j+1];
      }
    items[j+1] = "C";
    carHasMovedThisIteration = true;
    }
  }
}

However I think we have some difficulties with the variables being passed into the simulateCarMovement method, because of they are not being returned. So then the output for this refactored code would be:

Expected:
000 C...R............G......
001 .C..R............G......
002 ..C.R............G......
003 ...CR............G......
004 ...CR............G......
005 ....C............O......
006 ....GC...........R......
007 ....G.C..........R......
008 ....G..C.........R......
009 ....G...C........R......
010 ....O....C.......R......

Your result:
000 C...R............G......
001 ...CR............G......
002 ...CR............G......
003 ...CR............G......
004 ...CR............G......
005 ......................C.
006 ......................C.
007 ......................C.
008 ......................C.
009 ......................C.
010 ......................C.

A difference was detected at index 1

I have also read:

How could we refactor two related logics which are part of the same algorithm, into two separate methods‽

Aucun commentaire:

Enregistrer un commentaire