Checkstyle and SonarQube

src/edu/kit/informatik/model/Coach.java

@@ -2,6 +2,8 @@ package edu.kit.informatik.model;
 
 import edu.kit.informatik.ui.InvalidInputException;
 
+import java.util.regex.Pattern;
+
 /**
  * A coach.
  *
@@ -9,6 +11,15 @@ import edu.kit.informatik.ui.InvalidInputException;
  * @version 1.0
  */
 public abstract class Coach extends RollingStock {
+    /**
+     * String used to prefix coach identifiers.
+     */
+    public static final String IDENTIFIER_PREFIX = "W";
+    /**
+     * Regex pattern to match coach identifiers.
+     */
+    public static final Pattern IDENTIFIER_PATTERN = Pattern.compile(IDENTIFIER_PREFIX + "\\+?\\d+");
+
     /**
      * The unique identifier of this coach.
      */

src/edu/kit/informatik/model/ModelRailwaySimulation.java

@@ -95,8 +95,7 @@ public class ModelRailwaySimulation {
      */
     public boolean setSwitch(final int id, final Vector2D position) {
         boolean success = railNetwork.setSwitch(id, position);
-        if (success) {
+        if (success) { // derail trains on switch, explicitly not (!) printing any removed trains (source: forum post)
-            // derail trains on switch, explicitly not (!) printing any removed trains (source: forum post)
             trains.values().stream().filter(train -> train.isOnRail(id)).forEach(Train::removeFromRails);
         }
         return success;
@@ -124,12 +123,10 @@ public class ModelRailwaySimulation {
         } else {
             engines.sort(Comparator.comparing(Engine::getIdentifier));
             for (Engine engine : engines) {
-                Train train = trains.values().stream().filter(x -> x.contains(engine)).findFirst().orElse(null);
+                String trainId = trains.values().stream().filter(train -> train.contains(engine))
-                if (train != null) {
+                        .map(train -> Integer.toString(train.getIdentifier()))
-                    Terminal.printLine(String.format("%s %s", train.getIdentifier(), engine));
+                        .findFirst().orElse("none");
-                } else {
+                Terminal.printLine(String.format("%s %s", trainId, engine));
-                    Terminal.printLine(String.format("none %s", engine));
-                }
             }
         }
     }
@@ -183,15 +180,12 @@ public class ModelRailwaySimulation {
         } else {
             for (Integer identifier : coaches.keySet().stream().sorted().collect(Collectors.toList())) {
                 Coach coach = coaches.get(identifier);
-                Train train = trains.values().stream().filter(x -> x.contains(coach)).findFirst().orElse(null);
+                String trainId = trains.values().stream().filter(train -> train.contains(coach))
-                if (train != null) {
+                        .map(train -> Integer.toString(train.getIdentifier()))
+                        .findFirst().orElse("none");
                 Terminal.printLine(String.format("%d %s %s %d %b %b",
-                            coach.getNumericalIdentifier(), train.getIdentifier(), coach.getType(),
+                        coach.getNumericalIdentifier(), trainId, coach.getType(),
                         coach.getLength(), coach.hasCouplingFront(), coach.hasCouplingBack()));
-                } else {
-                    Terminal.printLine(String.format("%d none %s %d %b %b", coach.getNumericalIdentifier(),
-                            coach.getType(), coach.getLength(), coach.hasCouplingFront(), coach.hasCouplingBack()));
-                }
             }
         }
     }
@@ -219,12 +213,10 @@ public class ModelRailwaySimulation {
         } else {
             trainSets.sort(Comparator.comparing(TrainSet::getIdentifier));
             for (TrainSet trainSet : trainSets) {
-                Train train = trains.values().stream().filter(x -> x.contains(trainSet)).findFirst().orElse(null);
+                String trainId = trains.values().stream().filter(train -> train.contains(trainSet))
-                if (train != null) {
+                        .map(train -> Integer.toString(train.getIdentifier()))
-                    Terminal.printLine(String.format("%s %s", train.getIdentifier(), trainSet));
+                        .findFirst().orElse("none");
-                } else {
+                Terminal.printLine(String.format("%s %s", trainId, trainSet));
-                    Terminal.printLine(String.format("none %s", trainSet));
-                }
             }
         }
     }
@@ -236,14 +228,10 @@ public class ModelRailwaySimulation {
      */
     public boolean deleteRollingStock(final String id) {
         RollingStock rollingStock = getRollingStock(id);
-        if (rollingStock == null || trains.values().stream().anyMatch(train -> train.contains(rollingStock))) {
+        if (trains.values().stream().anyMatch(train -> train.contains(rollingStock))) {
-            // can not delete rolling stock in use
+            return false; // can not delete rolling stock in use
-            return false;
         }
-        engines.remove(rollingStock);
+        return engines.remove(rollingStock) || trainSets.remove(rollingStock) || coaches.values().remove(rollingStock);
-        trainSets.remove(rollingStock);
-        coaches.values().remove(rollingStock);
-        return true;
     }
 
     /**
@@ -252,7 +240,7 @@ public class ModelRailwaySimulation {
      * @return the specified rolling stock, or null if not found
      */
     public RollingStock getRollingStock(final String id) {
-        if (id.matches("W\\+?\\d+")) {
+        if (Coach.IDENTIFIER_PATTERN.matcher(id).matches()) {
             int coachId = Integer.parseInt(id.substring(1));
             return coaches.get(coachId);
         } else {
@@ -433,44 +421,7 @@ public class ModelRailwaySimulation {
                 nextOccupiedRails.put(train, train.getOccupiedRails());
             }
         });
-        // check for block collisions
+        checkForBlockCollisions(collisions, occupiedRails, nextOccupiedRails);
-        trains.values().stream().filter(train -> train.isPlaced()
-                && collisions.stream().noneMatch(x -> x.contains(train))).forEach(train -> {
-                    Set<Rail> occupiedByThisTrain = nextOccupiedRails.get(train);
-                    trains.values().stream().filter(x -> x != train).forEach(otherTrain -> {
-                        Set<Rail> occupiedByOtherTrainPreviously = occupiedRails.get(otherTrain);
-                        Set<Rail> occupiedByOtherTrain = nextOccupiedRails.get(otherTrain);
-                        if (occupiedByOtherTrain == null || occupiedByOtherTrainPreviously == null) {
-                            return;
-                        }
-                        boolean anyIntersection = occupiedByOtherTrain.stream().anyMatch(occupiedByThisTrain::contains)
-                                || occupiedByOtherTrainPreviously.stream().anyMatch(occupiedByThisTrain::contains);
-                        if (anyIntersection || train.touches(otherTrain)) {
-                            train.removeFromRails();
-                            otherTrain.removeFromRails();
-                            // try to find/merge existing collisions
-                            Set<Train> existingCollision = null;
-                            for (Set<Train> collision : collisions) {
-                                if (collision.contains(otherTrain) || collision.contains(train)) {
-                                    if (existingCollision == null) {
-                                        existingCollision = collision;
-                                        collision.add(train);
-                                        collision.add(otherTrain);
-                                    } else {
-                                        existingCollision.addAll(collision);
-                                        existingCollision.add(train);
-                                        existingCollision.add(otherTrain);
-                                        collisions.remove(collision);
-                                        break;
-                                    }
-                                }
-                            }
-                            if (existingCollision == null) {
-                                collisions.add(new HashSet<>(Arrays.asList(train, otherTrain)));
-                            }
-                        }
-                    });
-                });
         return collisions;
     }
 
@@ -504,38 +455,50 @@ public class ModelRailwaySimulation {
                 nextOccupiedRails.put(train, train.getOccupiedRails());
             }
         });
-        // check for block collisions
+        checkForBlockCollisions(collisions, occupiedRails, nextOccupiedRails);
-        for (Train train : trains.values()) {
+        return collisions;
-            if (!train.isPlaced() || collisions.stream().anyMatch(x -> x.contains(train))) {
-                continue; // already included in collision
     }
+    
+    private void checkForBlockCollisions(List<Set<Train>> collisions, Map<Train, Set<Rail>> occupiedRails,
+                                         Map<Train, Set<Rail>> nextOccupiedRails) {
+        trains.values().stream().filter(train -> train.isPlaced()
+                && collisions.stream().noneMatch(x -> x.contains(train))).forEach(train -> {
                     Set<Rail> occupiedByThisTrain = nextOccupiedRails.get(train);
                     trains.values().stream().filter(x -> x != train).forEach(otherTrain -> {
                         Set<Rail> occupiedByOtherTrainPreviously = occupiedRails.get(otherTrain);
                         Set<Rail> occupiedByOtherTrain = nextOccupiedRails.get(otherTrain);
-                if (occupiedByOtherTrain == null || occupiedByOtherTrainPreviously == null) {
+                        if (occupiedByOtherTrain == null) {
                             return;
                         }
-                boolean anyIntersection = occupiedByOtherTrain.stream().anyMatch(occupiedByThisTrain::contains)
+                        boolean anyIntersection = Stream.concat(
-                        || occupiedByOtherTrainPreviously.stream().anyMatch(occupiedByThisTrain::contains);
+                                occupiedByOtherTrain.stream(), occupiedByOtherTrainPreviously.stream())
-                if (anyIntersection) {
+                                .anyMatch(occupiedByThisTrain::contains);
+                        if (anyIntersection || train.touches(otherTrain)) {
                             train.removeFromRails();
+                            otherTrain.removeFromRails();
                             // try to find/merge existing collisions
                             Set<Train> existingCollision = null;
                             for (Set<Train> collision : collisions) {
-                        if (collision.contains(otherTrain)) {
+                                if (collision.contains(otherTrain) || collision.contains(train)) {
                                     if (existingCollision == null) {
                                         existingCollision = collision;
                                         collision.add(train);
+                                        collision.add(otherTrain);
                                     } else {
                                         existingCollision.addAll(collision);
+                                        existingCollision.add(train);
+                                        existingCollision.add(otherTrain);
+                                        collisions.remove(collision);
+                                        break;
                                     }
                                 }
                             }
+                            if (existingCollision == null) {
+                                collisions.add(Stream.of(train, otherTrain).collect(Collectors.toSet()));
+                            }
                         }
                     });
-        }
+                });
-        return collisions;
     }
 
     /**
@@ -552,16 +515,9 @@ public class ModelRailwaySimulation {
             return;
         }
 
-        List<Set<Train>> collisions;
+        List<Set<Train>> collisions = IntStream.range(0, Math.abs(speed))
-        if (speed >= 0) {
+                    .mapToObj(step -> speed >= 0 ? getCollisionsOfOneStep() : getCollisionsOfOneReverseStep())
-            collisions = IntStream.range(0, speed)
-                    .mapToObj(step -> getCollisionsOfOneStep())
                     .flatMap(List::stream).collect(Collectors.toList());
-        } else {
-            collisions = IntStream.range(0, -speed)
-                    .mapToObj(step -> getCollisionsOfOneReverseStep())
-                    .flatMap(List::stream).collect(Collectors.toList());
-        }
 
         for (int id : trains.keySet().stream().sorted().collect(Collectors.toList())) {
             Train train = trains.get(id);

src/edu/kit/informatik/ui/CommandLine.java

@@ -42,7 +42,7 @@ public final class CommandLine {
 
             if (input.startsWith(EXIT)) {
                 if (input.length() != EXIT.length()) {
-                    Terminal.printError("space after exit command");
+                    Terminal.printError("input after exit command");
                     continue;
                 } else {
                     break;

src/edu/kit/informatik/ui/command/CommandFactory.java

@@ -1,5 +1,6 @@
 package edu.kit.informatik.ui.command;
 
+import edu.kit.informatik.model.Coach;
 import edu.kit.informatik.ui.InvalidInputException;
 
 import java.util.HashMap;
@@ -29,7 +30,7 @@ public final class CommandFactory {
      * Regex to accept one rolling stock identifier.
      */
     public static final String ROLLING_STOCK_IDENTIFIER
-            = "(" + ALPHANUMERIC_WORD + "-" + ALPHANUMERIC_WORD + ")|W" + NUMBER;
+            = "(" + ALPHANUMERIC_WORD + "-" + ALPHANUMERIC_WORD + ")|" + Coach.IDENTIFIER_PATTERN;
 
     /**
      * Name of the add track command.

src/edu/kit/informatik/ui/command/CreateEngine.java

@@ -1,6 +1,7 @@
 package edu.kit.informatik.ui.command;
 
 import edu.kit.informatik.Terminal;
+import edu.kit.informatik.model.Coach;
 import edu.kit.informatik.model.DieselEngine;
 import edu.kit.informatik.model.ElectricalEngine;
 import edu.kit.informatik.model.Engine;
@@ -86,7 +87,7 @@ public class CreateEngine extends Command {
         }
         type = EngineType.parse(matcher.group(1));
         series = matcher.group(2);
-        if ("W".equals(series)) {
+        if (Coach.IDENTIFIER_PREFIX.equals(series)) {
             throw new InvalidInputException("invalid engine class/series");
         }
         name = matcher.group(3);

src/edu/kit/informatik/ui/command/CreateTrainSet.java

@@ -1,5 +1,6 @@
 package edu.kit.informatik.ui.command;
 
+import edu.kit.informatik.model.Coach;
 import edu.kit.informatik.model.ModelRailwaySimulation;
 import edu.kit.informatik.Terminal;
 import edu.kit.informatik.model.TrainSet;
@@ -64,7 +65,7 @@ public class CreateTrainSet extends Command {
             throw new InvalidInputException("invalid create train-set arguments");
         }
         series = matcher.group(1);
-        if ("W".equals(series)) {
+        if (Coach.IDENTIFIER_PREFIX.equals(series)) {
             throw new InvalidInputException("invalid train-set class/series");
         }
         name = matcher.group(2);

testcoverage_output.txt

@@ -38,11 +38,11 @@ OK
 2
 OK
 Crash of train 2
-Error, engine identifier already used by engine
+Error, engine identifier already used
 T2-Epsilon
-Error, engine identifier already used by train set
+Error, engine identifier already used
-Error, train set identifier already used by train set
+Error, train set identifier already used
-Error, train set identifier already used by engine
+Error, train set identifier already used
 T2-Lambda
 train-set T2-Lambda added to train 3
          ++         
@@ -129,7 +129,7 @@ Error, invalid train direction
 Error, invalid add track argument syntax
 Error, unknown command
 Error, invalid step argument
-Error, space after exit command
+Error, input after exit command
 OK
 OK
 OK