2016-04-18 7 views
1

Folgendes ist meine Code-Isolierung.Falsche Annäherung oder falsches OOP-Design?

Interaktive Schnittstelle.

public interface Interactable <E extends Interactable> { 

    List<Person> personsInteracting = new ArrayList<>(); 
    List<Person> personsWaiting  = new ArrayList<>(); 

    long INTERACTION_TIME = 5 * 60; 

    default int getNumberOfPeopleInteracting() { 
     return personsInteracting.size(); 
    } 

    default int getNumberOfPeopleWaiting() { 
     return personsWaiting.size(); 
    } 

    boolean isMultipleActionsAllowed(); 

    boolean isFurtherActionsAllowed(); 

    public abstract boolean tryOccupiedBy (final Person person, final Interactions interaction) 
     throws InteractionNotPossibleException; 

    E getObject(); 

    EnumSet<Interactions> getInteractions(); 
} 

Interactive Abstrakte Klasse

public abstract class InteractiveObject implements Interactable { 

    protected final String  name; 
    protected  int   numberOfSimultaneousInteractions; 
    protected  Interactions currentInteraction; 

    public InteractiveObject (final String name) { 
     this.name = name; 
    } 

    @Override 
    public boolean isMultipleActionsAllowed() { 
     return numberOfSimultaneousInteractions > 1; 
    } 

    @Override 
    public boolean isFurtherActionsAllowed() { 
     return personsInteracting.isEmpty() || 
       (getNumberOfPeopleInteracting() > numberOfSimultaneousInteractions); 
    } 

    @Override 
    public boolean tryOccupiedBy (final Person person, final Interactions interaction) 
     throws InteractionNotPossibleException { 
     boolean isOccupied = false; 
     if (!isFurtherActionsAllowed()) { 
      throw new InteractionNotPossibleException(this + " is already in use by some other " + 
                 "person."); 
     } 
     personsInteracting.add(person); 
     currentInteraction = interaction; 
     return isOccupied; 
    } 

    @Override 
    public String toString() { 
     return name; 
    } 

    public int getNumberOfSimultaneousInteractions() { 
     return numberOfSimultaneousInteractions; 
    } 
} 

Chair (Einer der Kinderklasse)

public class Chair extends InteractiveObject { 

    private final EnumSet<Interactions> INTERACTIONS = EnumSet.copyOf(Arrays.asList(
     new Interactions[] {Interactions.DRAG, Interactions.SIT})); 

    public Chair (final String objectName) { 
     super(objectName); 
     super.numberOfSimultaneousInteractions = 1; 
    } 

    @Override 
    public Interactable getObject() { 
     return this; 
    } 

    @Override 
    public EnumSet<Interactions> getInteractions() { 
     return INTERACTIONS; 
    } 
} 

Hier ist das Stück Code, das ausführt und bringt das Problem, diese Frage ist gefragt.

 final InteractiveObject chair1 = new Chair("Chair1"); 
     final Person   person1 = new Person("Person1"); 
     final Room    room = new Room("Room1", 2, 2); 
     room.personEnters(person1); 
     room.putObject(chair1); 
     person1.tryOccupying(chair1); 

Oberhalb Stück Code, erfolgreich besetzt das Stuhl Objekt. Nun

 final InteractiveObject chair2 = new Chair("Chair2"); 
     final Person   person2 = new Person("Person2"); 
     final Room    room2 = new Room("Room2", 2, 2); 
     room2.personEnters(person2); 
     room2.putObject(chair2); 
     person2.tryOccupying(chair2); 

Dieses Stück Code läßt nicht die person2 besetzen, da mein Code besagt, dass 1 Person bereits mit chair2 interagiert, wo da niemand mit ihm interagiert.

Lösung meines Problems:

zog ich meine Liste der personInteracting-InteractiveObject und Funktion tryOccupiedBy für jedes Kind Klasse und alles funktioniert.

Fragen:

  1. Ich habe personsInteracting in Interactable Schnittstelle, da ich, dass jede künftige Umsetzung von Interactable glauben, es haben wird. Entwickler müssen sich nicht selbst implementieren. (Aber vielleicht scheint diese Idee falsch zu sein)

  2. Wenn tryOccupiedBy Funktion dieselbe Implementierung hat, was ist der Zweck der gesamten OOP?

  3. Ich weiß jetzt, dass die Isolierung falsch war und ich weiß, wo die Stücke platziert werden, um die Ergebnisse zu erhalten. Aber kann mich jemand freundlich auf ein OOP-Konzept hinweisen, das ich nicht verstanden habe und das viel besser umgesetzt werden sollte?

Antwort

2

Das default Schlüsselwort wurde nicht auf die Java-Sprache hinzugefügt, um die Art der Sache zu tun, was Sie scheinen zu erreichen zu versuchen. Daten, die in einer Schnittstelle definiert sind, sollen konstant sein - die Modifikatoren 'öffentliche statische' werden automatisch auf alle Felddefinitionen in einer Schnittstelle angewendet. Wenn Sie in der Schnittstelle eine default Methode anlegen, muss diese entweder zustandslos sein oder direkt nur auf den rein statisch verfügbaren Zustand einwirken. Standardmethoden können andere Schnittstellenmethoden aufrufen, um den Instanzstatus zu ändern.

Indem Sie das Feld personsInteracting in die Schnittstelle einfügen, haben Sie jedem Objekt, das diese Schnittstelle implementiert, die gleiche Instanz zugewiesen, sodass Ihre tryOccupying-Methode auf einen rein globalen Status angewendet wurde.

Also, der Zweck der default Methoden in der Sprache Java ist das Hinzufügen neuer Methoden zu Schnittstellen in einer abwärtskompatiblen Weise, nichts mehr zu unterstützen. Sie sollten es nicht als generische Form der Code-Wiederverwendung verwenden - es war nie dafür gedacht und Sie werden (wie Sie es taten) komisches Verhalten bekommen.

Sie haben nicht haben tryOccupiedBy in den Kinderklassen zu setzen, aber so haben Sie nicht haben eine Last von duplizierten Code zu haben. Sie könnten die Methodensignatur immer noch in der Schnittstelle deklarieren (was die Schnittstellen normalerweise tun sollen) und dann die allgemeine Methode in Ihrer abstrakten Basisklasse implementieren. Wenn Sie die Datenfelder in die Basisklasse setzen, werden sie in die Felder instance umgewandelt, sodass sie nicht zwischen Objekten aufgeteilt werden.

public interface Interactable <E extends Interactable> { 
    ... 
    boolean tryOccupiedBy (final Person person, final Interactions interaction) 
     throws InteractionNotPossibleException; 
    ... 
} 

public abstract class InteractiveObject implements Interactable { 
    private final List<Person> personsInteracting = new ArrayList<>(); 
    private final List<Person> personsWaiting  = new ArrayList<>(); 
    ... 
    @Override 
    public final boolean tryOccupiedBy (final Person person, final Interactions interaction) 
     throws InteractionNotPossibleException { 
     boolean isOccupied = false; 
     if (!isFurtherActionsAllowed()) { 
      throw new InteractionNotPossibleException(this + " is already in use by some other " + 
                 "person."); 
     } 
     personsInteracting.add(person); 
     currentInteraction = interaction; 
     return isOccupied; 
    } 
    ... 
} 
+0

Guter Punkt, Antwort bearbeitet zu enthalten. – sisyphus