Input/Output Zwischen zwei ID-Räumen unterscheiden und Objekt löschen

[DagobertDuck]

Ich arbeite derzeit an einem Projekt zur Eisenbahnsimulation. RollingStock (Rollmaterial) ist die Oberklasse und Coach (Wagen), Engine (Lokomotive) und TrainSet (Triebzug) sind Unterklassen von RollingStock.

- Alle Wagen haben eine eindeutige ganzzahlige aufsteigende ID, die bei 1 beginnt.
- TrainSets und Engines teilen sich ihren ID-Raum. Diese ID besteht aus einer Baureihe (series) und einem Namen (name) (beides Strings).

Nun muss ich den Befehl delete rolling stock <id> implementieren.
Das Rollmaterial wird durch seine eindeutige ID identifiziert. Bei einem Wagen wird der ID ein W vorangestellt. Die Eingabe der ID für TrainSets/Engines ist <series>-<name>.

Beispiele für eine gültige Benutzereingabe wären also
- delete rolling stock W42, Coach mit der ID 42 wird gelöscht,
- delete rolling stock ICE-1106, TrainSet oder Engine mit der Baureihe ICE und dem Namen 1106 wird gelöscht.

Wie bei allen anderen meiner Befehlen habe ich wie folgt begonnen:

public class DeleteRollingStockCommand extends Command {
    private static final Pattern PATTERN = Pattern.compile(
            String.format("delete rolling stock%s(?<id>%s)", InOutputStrings.COMMAND_SEPARATOR,
                    InOutputStrings.ROLLING_STOCK_ID_PATTERN));
    private String[] id;
    
    @Override
    public void execute() {
        try {
            // TODO: Rollmaterial mit angegebener ID löschen
            // RollingStock rollingStock = register.getRollingStock(id);
            // register.removeRollingStock(rollingStock);
            Terminal.printLine(InOutputStrings.OK);
        } catch (InvalidInputException e) {
            Terminal.printError(e.getMessage());
        }
    }
    
    @Override
    void setArguments(String argument) {
        Matcher matcher = PATTERN.matcher(argument);
        // TODO: Methode parseRollingStockId() programmieren
        // id = parseRollingStockId(matcher.group("id"));
    }

(COMMAND_SEPARATOR = " ", ROLLING_STOCK_ID_PATTERN = "([W]+?[0-9]+)|(\w+-\w+)")

Ich bin mir leider unsicher, wie ich diesen Befehl am geschicktesten implementieren kann.

Damit ihr etwas mehr die Struktur meines Codes versteht:

Coaches, TrainSets und Engines werden wie folgt gespeichert:

private final Map<Integer, Coach> coaches;

private final List<Engine> engines;
private final List<TrainSet> trainSets;

private final Map<Integer, Coach> coaches;

private final List<Engine> engines;

private final List<TrainSet> trainSets;

    public MyClassName() {
        this.coaches = new TreeMap<>();
        this.engines = new ArrayList<>();
        this.trainSets = new ArrayList<>();
    }

 

[mihe7]

Wichtig sind zwei Dinge. Den ersten hast Du schon genannt:

Das Rollmaterial wird durch seine eindeutige ID identifiziert.

Eine ID ist einfach etwas, das ein Objekt eindeutig identifiziert, ob sich diese aus anderen Daten zusammensetzt, spielt keine Rolle.

Der zweite Punkt wäre, woraus das Rollmaterial entfernt werden soll. Wenn Du das weißt, kannst Du der betreffenden Klasse eine entsprechende Methode spendieren, die als Parameter eben die ID erhält.

 

[DagobertDuck]

Danke für die Antwort. Allerdings geht es mir eigentlich eher um die Verarbeitung der Benutzereingabe, also wie zwischen den eingegebenen IDs unterschieden werden kann. Außerdem soll dann das W entfernt werden, der String in ein integer umgewandelt werden bzw. die ID in zwei Strings aufgeteilt werden (series, name). Wie ich das genau machen soll ist mir unklar, insbesondere welcher Datentyp geeignet wäre.

 

[mihe7]

Allerdings geht es mir eigentlich eher um die Verarbeitung der Benutzereingabe, also wie zwischen den eingegebenen IDs unterschieden werden kann.

OK, ich habe das vielleicht nicht deutlich genug formuliert. Die Unterscheidung in W<zahl> und <series>-<name> misst der ID eine Bedeutung zu. Eine ID hat aber keine Bedeutung, es ist einfach irgendein String. Ein RollingStock ist etwas, das eine ID besitzt, fertig.

Aus dem Bauch raus (kann Fehler enthalten):

public class RollingStockRepository {
    Map<String, RollingStock> stocks = new HashMap<>();

    public void add(RollingStock rs) { stocks.put(rs.getId(), rs); }
    public void delete(String id) { stocks.remove(id); }

    public <T extends RollingStock> get(String id, Class<T> clazz) {
        RollingStock rs = stocks.get(id);
        if (rs != null && clazz.isInstance(rs)) { 
            return clazz.cast(rs);
        }
        return null; // oder Exception
    }
}

Um Deine Frage trotzdem zu beantworten:

if (str.contains("-")) {
     // str muss der Form <series>-<name> entsprechen
} else if (str.startsWith("W")) {
     // str muss der Form W<zahl>entsprechen
     int wagonId = Integer.parseInt(str.substring(1));
}

bzw. die ID in zwei Strings aufgeteilt werden (series, name)

Wozu das denn?

 

[DagobertDuck]

Wozu das denn?

Da der Konstruktor von Coach momentan so aussieht Coach(int id, int length, ...) und der Konstruktor von TrainSet und Engine folgendermaßen aussieht TrainSet(String series, String name, int length, ...) bzw. Engine(String series, String name, int length, ...). RollingStock besitzt also gar keine ID, sondern nur die Unterklassen. Das liegt daran, dass es zwei verschiedene ID-Räume gibt.
Um dann überprüfen zu können, ob die ID überhaupt existiert, muss ich zuerst den String in Series und Name teilen.

Eine eigene Map nur für RollingStock habe ich außerdem auch nicht.
Ist meine jetzige Modellierung ausbaufähig, und wie gehe ich das Problem dann am besten an?

 

[mihe7]

RollingStock besitzt also gar keine ID, sondern nur die Unterklassen. Das liegt daran, dass es zwei verschiedene ID-Räume gibt.

Die Räume spielen genauso wenig eine Rolle wie die Konstuktoren: RollingStock beschreibt etwas, das identifizierbar ist.

Möglichkeit 1:

abstract class RollingStock {
    ...
    abstract String getId();
}

Möglichkeit 2:

abstract class RollingStock {
    private String id;

    protected RollingStock(String id) { this.id = id; }
  
    public String getId() { return id; }
}

In z. B. Coach:

class Coach extends RollingStock {
    private int coachNo;
     public Coach(int coachNo, int length, ...) {
         super("W" + coachNo); // Möglichkeit 2
         this.coachNo = coachNo;
         // ...
     }

     // Möglichkeit 1
    @Override
    public String getId() {
         return "W" + coachNo;
    }
}

 

[DagobertDuck]

Es sieht bei mir jetzt so aus:

public abstract class Coach extends RollingStock {
    private final int id;

    public Coach(..., int id) {
        super(...);
        this.id = id;
    }

    public String getId() {
        return Integer.toString(id);
    }
}

und

public abstract class Engine extends RollingStock {
    private String series;
    private String name;

    public Engine(..., String series, String name) {
        super(...);
        this.series = series;
        this.name = name;
    }

    public String getSeries() {
        return series;
    }

    public String getName() {
        return name;
    }

    public String getId() {
        return series + "-" + name;
    }
}

Analog für TrainSet.

Jetzt frage ich mich allerdings wie die execute() Methode des Befehls auszusehen hat.

    @Override
    public void execute() {
        if (id.startsWith("W")) {
            ...
        } else if (id.contains("-")) {
           ...
        }
    }

Was ist z. B. wenn die Baureihe (series) mit "W" anfängt? Und wie bekomme ich jetzt das RollingStock mit dieser ID?
For each Schleife über RollingStock geht nicht, da ich wie gesagt nur folgende Listen/Maps habe:

    private final Map<Integer, Coach> coaches;
    private final List<Engine> engines;
    private final List<TrainSet> trainSets;

 

[mrBrown]

Sind die drei Listen/Maps so vorgegeben?


Du kannst einfach "blind" aus allen dreien löschen. Wenn du versuchst, den Coach aus engines zu löschen, passiert halt einfach nichts, erst wenn du den Coach aus coaches löscht, wird es auch gelöscht (und analog zu allen anderen).

Mit der ID selber solltest du im Idealfall nichts machen - das ist einfach nur ein String wie der aufgebaut ist, ist völlig egal.

Es sieht bei mir jetzt so aus:

public abstract class Coach extends RollingStock {
    private final int id;

    public Coach(..., int id) {
        super(...);
        this.id = id;
    }

    public String getId() {
        return Integer.toString(id);
    }
}

Die ID sollte doch mit "W" beginnen?

 

[DagobertDuck]

Ich glaube ich habe etwas für Verwirrung gesorgt.

Die ID soll nur bei der Benutzereingabe mit "W" beginnen. Wenn allerdings z. B. der Befehl "list coaches" aufgerufen wird, soll die ID ohne "W" ausgegeben werden.

Problematisch wird es dann auch mit dem createCoach Befehl. Momentan wird ein Wagen hier abgelegt Map<Integer, Coach> coaches.

    public void createCoach(Coach coach) {
        this.coaches.put(coach.getId(), coach);
    }

Leider bin ich etwas ungewiss, wie ich jetzt weiter machen soll...

 

[mihe7]

Mach aus Map<Integer, Coach> halt Map<String, Coach>.

 

[DagobertDuck]

Das ist aber doch nicht schön. Außerdem funktionieren dann all meine anderen Methoden wie z. B. getNextId() nicht mehr. Kann man den Command nicht schöner lösen? Also so, dass Coach immer noch int id und TrainSet/Engine String series, String name als Attribute haben.

 

[mihe7]

Kann man den Command nicht schöner lösen?

Schöner liegt im Auge des Betrachters. Du kannst alles so lassen, wie Du es hattest und dann mit dem Code aus #4 ("Um Deine Frage dennoch zu beantworten") arbeiten.

Schöner fände ich allerdings, wenn Du Deine ID anpassen würdest.

 

[DagobertDuck]

@mihe7 OK. Wäre es also sinnvoller eine einzige Map<String, RollingStock> rollingStock zu erstellen, in der Coaches, TrainSets und Engines gespeichert werden? Dann frage ich mich allerdings, wie ich z. B. beim "list coaches" Befehl nur Coaches ausgeben soll.

Dadurch wäre die Implementierung des "delete rolling stock <id>" und des "add <trainID> <rollingStockID>" Befehls deutlich einfacher.

 

[mihe7]

Wäre es also sinnvoller eine einzige Map<String, RollingStock> rollingStock zu erstellen, in der Coaches, TrainSets und Engines gespeichert werden?

Das wäre damit zumindest möglich, muss aber nicht unbedingt sein. Wichtig war, dass die IDs aller RollingStock-Objekte vom selben Typ sind. Damit entfällt die lästige Unterscheidung, was die ID betrifft. Die Map wäre nur ein "Index", um anhand des (nun einheitlichen) Schlüssels an das betreffende Objekt zu kommen.

Dann frage ich mich allerdings, wie ich z. B. beim "list coaches" Befehl nur Coaches ausgeben soll.

Zum Beispiel, indem Du die Elemente des gesuchten Typs rausfilterst oder zusätzliche Listen verwaltest.

Die Filter-Variante würde im RollingStockRepository aus Kommentar #4 so aussehen:

public <T extends RollingStock> List<T> findByType(Class<T> type) {
    return stock.values().stream()
        .filter(e -> e.getClass().equals(type))
        .map(clazz::cast)
        .collect(Collectors.toList());
}

Damit kannst Du über das Repository nun RollingStock-Objekte hinzufügen, anhand ihrer ID löschen und nach Typ auflisten:

RollingStockRepository repos = new RollingStockRepository();
repos.add(new Coach(...));
// ...
List<Coach> coaches = repos.findByType(Coach.class);

 

[DagobertDuck]

OK, vielen Dank. Ich habe jetzt erstmal RollingStock und seinen Unterklassen das Attribut String id hinzugefügt.

Die drei Listen werde ich allerdings behalten und nicht eine einzige Liste für RollingStock benutzen.

Bei manchen Befehlen muss ich die Baureihe und den Namen eines TrainSets / einer Engine getrennt ausgeben.

Wenn ich .split("-") benutze kann es auch zu Fehlern kommen, da series und name jeweils auch das Zeichen "-" enthalten dürfen.

Wie kann ich nun folgende Methoden implementieren?

    public String getSeries() {
         ...
        return series;
    }

    public String getName() {
        ...
        return name;
    }

Gibt es denn keinen anderen Weg, ohne RollingStock eine id zu geben? Es ist doch schöner, wenn Coach direkt eine id vom Typ Integer hat und TrainSet/Engine getrennt String series und String name besitzen..

 

[mrBrown]

Du brauchst kein extra Attribut für die ID, ein Getter dafür reicht.
In dem Getter kannst du die ID notfalls passendes zusammenbauen, zB aus series und Name, die normale Attribute bleiben können.

 

[mihe7]

Es ist doch schöner, wenn Coach direkt eine id vom Typ Integer hat und TrainSet/Engine getrennt String series und String name besitzen..

Coach hat eine Wagennummer, die vom Typ int sein kann und TrainSet/Engine haben z. B. series und name als Attribute; beide haben eine ID vom Typ String.

Wie das ginge, habe ich zwar in Kommentar #6 schon gezeigt aber mal konkreter für beides:

abstract class RollingStock {
    ...
    abstract String getId();
}

class Coach extends RollingStock {
     private final int coachNo;

     public Coach(int coachNo, int length, ...) {
         super(...);
         this.coachNo = coachNo;
     }

    public int getCoachNo() { return coachNo; }

    @Override
    public String getId() {
         return "W" + coachNo;
    }
}

class TrainSet extends RollingStock {
     private final String series;
     private final String name;

     public Coach(String series, String name, int length, ...) {
         super(...);
         this.series = series;
         this.name = name;
     }

    public String getName() { return name; }
    public String getSeries() { return series; }

    @Override
    public String getId() {
         return series + "-" + name;
    }
}