2016-04-18 3 views
1

Ниже приведена моя изоляция кода.Неправильный подход или неправильный дизайн ООП?

Взаимодействующий интерфейс.

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(); 
} 

InteractiveObject Абстрактный класс

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; 
    } 
} 

Председатель (один из дочернего класса)

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; 
    } 
} 

Вот фрагмент кода, который выполняет и приносит проблему, задается этот вопрос.

 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); 

Над фрагментом кода, успешно занимает объект стула. Теперь

 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); 

Этот фрагмент кода не позволяет person2 занимать, так как мой код утверждает, что 1 человек уже взаимодействует с chair2, где никто не взаимодействует с ним.

Решение моей проблемы:

Я переехал мой список personInteracting к InteractiveObject и функциям tryOccupiedBy к каждому классу ребенка, и все работает отлично.

Вопросы:

  1. Я положил personsInteracting в Interactable интерфейс, так как я считаю, что каждый будущее осуществление Interactable будет иметь. Разработчикам не придется реализовывать себя. (Но, возможно, эта идея кажется неправильной)

  2. Если функция tryOccupiedBy имеет такую ​​же реализацию, какова цель всего ООП?

  3. Теперь я знаю, что изоляция была неправильной, и я знаю, где разместить фигуры, чтобы получить результаты. Но может кто-то любезно указать мне на какую-то концепцию ООП, которую я не понимал и должен быть реализован гораздо лучше?

ответ

2

default ключевое слово не было добавлено к языку Java, чтобы сделать такие вещи, которые вы, кажется, пытается достичь. Данные, определенные в интерфейсе, должны быть постоянными - публичные статические переменные модификаторов автоматически применяются к любым определениям полей в интерфейсе. Если вы создаете метод default в интерфейсе, он должен либо быть без гражданства, либо действовать непосредственно только в состоянии, имеющем чисто статическое состояние. Методы по умолчанию могут вызывать другие методы интерфейса для изменения состояния экземпляра.

Поместив в интерфейс поле personsInteracting, вы сделали тот же экземпляр, общий для каждого объекта, реализующего этот интерфейс, и поэтому ваш метод tryOccupying действовал исключительно в глобальном состоянии.

Таким образом, целью использования методов default на языке Java является поддержка добавления новых методов в интерфейсы в обратном режиме, не более того. Вы не должны повторно использовать его в качестве общей формы повторного использования кода - он никогда не предназначался для этого, и вы получите (как вы) странное поведение.

Вы не имеют поставить tryOccupiedBy в дочерних классах, однако, так что вы не имеют иметь нагрузку дублированного кода. Вы все равно можете объявить подпись метода в интерфейсе (который, как правило, должен выполнять интерфейс), а затем реализовать общий метод в вашем абстрактном базовом классе. Помещая поля данных в базовый класс, вы делаете их экземплярами полей, и поэтому они не разделяются между объектами.

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

Хорошая точка, ответ отредактирован для включения. – sisyphus

Смежные вопросы