rails-commits

[Rails-commits] Branch 'rails2.0' - junit/rails src/rails

[Stefan F. <ste...@us...>]

 junit/rails/game/state/FormatterTest.java      |   54 +++++++++++++------
 junit/rails/game/state/GenericStateTest.java   |    3 -
 junit/rails/game/state/ObservableTest.java     |    2 
 junit/rails/game/state/ObserverTest.java       |   37 +++++++++++++
 junit/rails/game/state/StateTest.java          |    8 +-
 src/rails/game/GameManager.java                |    2 
 src/rails/game/OperatingRound.java             |    2 
 src/rails/game/PublicCompany.java              |    2 
 src/rails/game/model/PresidentModel.java       |   11 ---
 src/rails/game/state/Change.java               |    2 
 src/rails/game/state/ChangeSet.java            |   12 +++-
 src/rails/game/state/ChangeStack.java          |   28 ++++++++--
 src/rails/game/state/Formatter.java            |   22 ++++++-
 src/rails/game/state/Model.java                |   31 ++++++-----
 src/rails/game/state/Observable.java           |   35 ++++++------
 src/rails/game/state/Observer.java             |    2 
 src/rails/game/state/Root.java                 |   13 ++--
 src/rails/game/state/State.java                |   32 ++---------
 src/rails/game/state/StateManager.java         |   70 ++++++++++++++++++++++++-
 src/rails/ui/swing/GridPanel.java              |    2 
 src/rails/ui/swing/elements/Field.java         |    4 -
 src/rails/ui/swing/elements/GUIStockSpace.java |    3 -
 src/rails/ui/swing/hexmap/GUIHex.java          |    9 ---
 23 files changed, 260 insertions(+), 126 deletions(-)

New commits:
commit afd1a0667ca3e872149f6657eec7e9b676338f70
Author: Stefan Frey <ste...@we...>
Date:   Wed Jul 11 11:35:02 2012 +0200

    refactored Observer and Formatter classes

diff --git a/junit/rails/game/state/FormatterTest.java b/junit/rails/game/state/FormatterTest.java
index 626f0be..51f6b48 100644
--- a/junit/rails/game/state/FormatterTest.java
+++ b/junit/rails/game/state/FormatterTest.java
@@ -1,40 +1,60 @@
 package rails.game.state;
 
-import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InOrder;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
 
+@RunWith(MockitoJUnitRunner.class)
 public class FormatterTest {
+    
+    private static final String YES = "yes";
+    private static final String NO = "no";
 
     // This formatter doubles the text of the state
-    private class FormatterImpl implements Formatter<State> {
-        public String formatValue(State state) {
-            return state.toString() + state.toString();
+    private class FormatterImpl extends Formatter<BooleanState> {
+        private final BooleanState state;
+        private FormatterImpl(BooleanState state){
+            super(state);
+            this.state = state;
+        }
+        public String observerText() {
+            if (state.value()) {
+                return YES;
+            } else {
+                return NO;
+            }
         }
     }
     
     private final static String STATE_ID = "State";
-    private final static String STATE_TEXT = "Test";
     
     private Root root;
-    private State state;
-    private Formatter<State> formatter;
-    
+    private BooleanState state;
+    private Formatter<BooleanState> formatter;
+    @Mock private Observer observer;
+
     @Before
     public void setUp() {
-        root = Root.create();
-        state = new StateImpl(root, STATE_ID, STATE_TEXT);
-        formatter = new FormatterImpl();
+        root = StateTestUtils.setUpRoot();
+        state = BooleanState.create(root, STATE_ID);
+        formatter = new FormatterImpl(state);
+        formatter.addObserver(observer);
     }
     
     @Test
     public void testFormatValue() {
-        assertEquals(STATE_TEXT, state.getText());
-        state.setFormatter(formatter);
-        assertEquals(STATE_TEXT + STATE_TEXT, state.getText());
-        state.setFormatter(null);
-        assertEquals(STATE_TEXT, state.getText());
-    }
+        state.set(true);
+        root.getStateManager().getChangeStack().closeCurrentChangeSet();
+        state.set(false);
+        root.getStateManager().getChangeStack().closeCurrentChangeSet();
 
+        InOrder inOrder = inOrder(observer); 
+        inOrder.verify(observer).update(YES);
+        inOrder.verify(observer).update(NO);
+    }
 }
diff --git a/junit/rails/game/state/GenericStateTest.java b/junit/rails/game/state/GenericStateTest.java
index c25424c..36926f9 100644
--- a/junit/rails/game/state/GenericStateTest.java
+++ b/junit/rails/game/state/GenericStateTest.java
@@ -66,7 +66,8 @@ public class GenericStateTest {
         assertSame(another_item, state_init.get());
 
         stack.closeCurrentChangeSet();
-        assertThat(stack.getLastClosedChangeSet().getStates()).contains(state_default, state_init);
+        // remark: state_init is an internal (isObservable = false)
+        assertThat(stack.getLastClosedChangeSet().getStates()).contains(state_default);
         
         stack.undo();
         assertNull(state_default.get());
diff --git a/junit/rails/game/state/ObservableTest.java b/junit/rails/game/state/ObservableTest.java
index 2e2000b..98d96f3 100644
--- a/junit/rails/game/state/ObservableTest.java
+++ b/junit/rails/game/state/ObservableTest.java
@@ -20,7 +20,7 @@ public class ObservableTest {
         }
 
         @Override
-        public String getText() {
+        public String observerText() {
             return null;
         }
     }
diff --git a/junit/rails/game/state/ObserverTest.java b/junit/rails/game/state/ObserverTest.java
new file mode 100644
index 0000000..94b0671
--- /dev/null
+++ b/junit/rails/game/state/ObserverTest.java
@@ -0,0 +1,37 @@
+package rails.game.state;
+
+import static org.mockito.Mockito.*;
+import static org.fest.assertions.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ObserverTest {
+    
+    private final static String STATE_ID = "State";
+    
+    private Root root;
+    private BooleanState state;
+    @Mock private Observer observer;
+
+    @Before
+    public void setUp() {
+        root = StateTestUtils.setUpRoot();
+        state = BooleanState.create(root, STATE_ID, false);
+        state.addObserver(observer);
+    }
+    
+    @Test
+    public void testUpdate() {
+        assertThat(state.getObservers()).contains(observer);
+        state.set(true);
+        verify(observer, never()).update(state.observerText());
+        root.getStateManager().getChangeStack().closeCurrentChangeSet();
+        verify(observer).update(state.observerText());
+    }
+
+}
diff --git a/junit/rails/game/state/StateTest.java b/junit/rails/game/state/StateTest.java
index 61bffab..0fb56a5 100644
--- a/junit/rails/game/state/StateTest.java
+++ b/junit/rails/game/state/StateTest.java
@@ -46,9 +46,9 @@ public class StateTest {
     }
     
     @Test
-    public void testGetText() {
-        assertNull(state.getText());
-        assertEquals(STATE_TEXT, state_wo_id.getText());
-    }
+            public void testObserverText() {
+                assertNull(state.observerText());
+                assertEquals(STATE_TEXT, state_wo_id.observerText());
+            }
 
 }
diff --git a/src/rails/game/GameManager.java b/src/rails/game/GameManager.java
index acb3453..083fba6 100644
--- a/src/rails/game/GameManager.java
+++ b/src/rails/game/GameManager.java
@@ -782,7 +782,7 @@ public class GameManager extends AbstractItem implements ConfigurableComponent,
     }
 
     public String getNumOfORs () {
-        return numOfORs.getText();
+        return numOfORs.observerText();
     }
 
     /* (non-Javadoc)
diff --git a/src/rails/game/OperatingRound.java b/src/rails/game/OperatingRound.java
index 50becdd..b5b56ad 100644
--- a/src/rails/game/OperatingRound.java
+++ b/src/rails/game/OperatingRound.java
@@ -3124,7 +3124,7 @@ public class OperatingRound extends Round implements Observer {
     /**
      * Update the status if the step has changed by an Undo or Redo
      */
-    public void update(Observable observable, String text) {
+    public void update(String text) {
         prepareStep();
     }
 
diff --git a/src/rails/game/PublicCompany.java b/src/rails/game/PublicCompany.java
index 30f7527..683c471 100644
--- a/src/rails/game/PublicCompany.java
+++ b/src/rails/game/PublicCompany.java
@@ -1191,7 +1191,7 @@ public class PublicCompany extends Company implements CashOwner, PortfolioOwner,
     }
 
     public String getFormattedCash() {
-        return treasury.getText();
+        return treasury.observerText();
     }
 
     public Model getText() {
diff --git a/src/rails/game/model/PresidentModel.java b/src/rails/game/model/PresidentModel.java
index 0f7cbb4..f167c72 100644
--- a/src/rails/game/model/PresidentModel.java
+++ b/src/rails/game/model/PresidentModel.java
@@ -3,18 +3,16 @@ package rails.game.model;
 import rails.game.Player;
 import rails.game.PublicCompany;
 import rails.game.state.Model;
-import rails.game.state.Observable;
-import rails.game.state.Observer;
 
 /**
  * model object for the current company president
  * gets registered by the ShareModels
  * 
- * FIXME: Finalize implementation
+ * FIXME: Finalize implementation, this does not work currently
  * TODO: Check if this is all done correctly, where is the observable stored?
  */
 
-public final class PresidentModel extends Model implements Observer {
+public final class PresidentModel extends Model {
 
     public static final String ID = "PresidentModel";  
     
@@ -43,9 +41,4 @@ public final class PresidentModel extends Model implements Observer {
         else return company.getPresident().getNameAndPriority();
     }
 
-    // FIXME: Add code what to do here
-    public void update(Observable observable, String text) {
-        
-    }
-
 }
diff --git a/src/rails/game/state/Change.java b/src/rails/game/state/Change.java
index eecaafc..c01bdcb 100644
--- a/src/rails/game/state/Change.java
+++ b/src/rails/game/state/Change.java
@@ -7,7 +7,7 @@ package rails.game.state;
 abstract class Change {
     
     protected void init(State state){
-        state.getRoot().getStateManager().getChangeStack().getCurrentChangeSet().addChange(this);
+        state.getStateManager().getChangeStack().getCurrentChangeSet().addChange(this);
     }
 
     abstract void execute();
diff --git a/src/rails/game/state/ChangeSet.java b/src/rails/game/state/ChangeSet.java
index d57c599..5c4d0c3 100644
--- a/src/rails/game/state/ChangeSet.java
+++ b/src/rails/game/state/ChangeSet.java
@@ -58,7 +58,9 @@ abstract class ChangeSet {
     private void defineStates() {
         ImmutableSet.Builder<State> builder = new ImmutableSet.Builder<State>();
         for (Change change:changes) {
-            builder.add(change.getState());
+            if (change.getState().isObservable()){
+                builder.add(change.getState());
+            }
         }
         states = builder.build();
     }
@@ -92,8 +94,16 @@ abstract class ChangeSet {
     
     abstract boolean isTerminal();
 
+    /**
+     * @return set of States in the ChangeSet
+     * Remark: It only includes those which are observable (isObservable = true)
+     */
     ImmutableSet<State> getStates() {
         if (!closed) throw new IllegalStateException("ChangeSet is still open");
         return states;
     }
+    
+    
+
+    
 }
diff --git a/src/rails/game/state/ChangeStack.java b/src/rails/game/state/ChangeStack.java
index b3585e6..0d2687a 100644
--- a/src/rails/game/state/ChangeStack.java
+++ b/src/rails/game/state/ChangeStack.java
@@ -2,10 +2,14 @@ package rails.game.state;
 
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.Sets;
+
 import rails.game.Player;
 import rails.game.action.PossibleAction;
 
@@ -18,12 +22,16 @@ public class ChangeStack {
 
     private final Deque<ChangeSet> undoStack = new ArrayDeque<ChangeSet>();
     private final Deque<ChangeSet> redoStack = new ArrayDeque<ChangeSet>();
+    private final StateManager stateManager;
+
     private ChangeSet currentSet;
 
-    private ChangeStack() {};
+    private ChangeStack(StateManager stateManager) {
+        this.stateManager = stateManager;
+    }
     
-    public static ChangeStack create() {
-        ChangeStack changeStack = new ChangeStack();
+    public static ChangeStack create(StateManager stateManager) {
+        ChangeStack changeStack = new ChangeStack(stateManager);
         changeStack.startAutoChangeSet(true); // first set is terminal
         return changeStack;
     }
@@ -50,6 +58,8 @@ public class ChangeStack {
             return false;
         } else {
             currentSet.close();
+            // FIXME: This is a hack, has to replaced
+            updateObservers(Sets.newHashSet(currentSet));
         }
         undoStack.addFirst(currentSet);
         return true;
@@ -139,11 +149,21 @@ public class ChangeStack {
     }
     
     /**
+     * Update the relevant observers
+     */
+    private void updateObservers(Set<ChangeSet> changeSets) {
+        Set<State> states = Sets.newHashSet();
+        for (ChangeSet cs:changeSets) {
+            states.addAll(cs.getStates());
+        }
+        stateManager.updateObservers(states);
+    }
+    
+    /**
      * @return size of ChangeStack
      */
     public int sizeUndoStack() {
         return undoStack.size();
     }
     
-    
 }
diff --git a/src/rails/game/state/Formatter.java b/src/rails/game/state/Formatter.java
index 223fab8..c0b5ab5 100644
--- a/src/rails/game/state/Formatter.java
+++ b/src/rails/game/state/Formatter.java
@@ -1,9 +1,23 @@
 package rails.game.state;
 
 /**
- * An interface defining a formatter for a state variable
- * @author freystef
+ * Abstract class for a Formatter 
  */
-public interface Formatter<E extends State> {
-    public String formatValue(E state);
+public abstract class Formatter<T extends Observable> {
+    
+    private final T observable;
+    
+    protected Formatter(T observable) {
+        this.observable = observable;
+    }
+    
+    public abstract String observerText();
+    
+    public void addObserver(Observer observer) {
+        observable.getStateManager().addObserver(observer, this);
+    }
+    
+    public T getObservable() {
+        return observable;
+    }
 }
diff --git a/src/rails/game/state/Model.java b/src/rails/game/state/Model.java
index d774e11..2143924 100644
--- a/src/rails/game/state/Model.java
+++ b/src/rails/game/state/Model.java
@@ -1,6 +1,5 @@
 package rails.game.state;
 
-
 /**
  * Model is an abstract generic class
  * that defines the a middle layer between State(s) and
@@ -8,14 +7,12 @@ package rails.game.state;
  * 
  * Models themselves can be layered upon each other.
  * 
- * 
  * It replaces the ModelObject class in Rails 1.0 
- * 
- * @author freystef
  */
+
 public abstract class Model extends Observable {
 
-    private boolean updated = false;
+    private boolean current = false;
     private String cache = null;
     
     protected Model(Item parent, String id) {
@@ -23,24 +20,30 @@ public abstract class Model extends Observable {
     }
 
     /**
-     * Indicates that the model is updated, so the getText() cache
-     * is flushed
+     * Calling of update informs the model that some prerequisites has been updated
      */
     public void update() {
-        updated = false;
+        current = false;
     }
     
     /**
-     * For a model the text shown to observer is derived from toString()
-     * The value is cached until the model is updated
+     * {@inheritDoc}
+     * Remark: A Model has to either override this or cachedText(), the latter automatically caches results
      */
     @Override
-    public final String getText() {
-        if (!updated){
-            updated = true;
-            cache = toString();
+    public String observerText() {
+        if (!current){
+            current = true;
+            cache = this.cachedText();
         }
         return cache;
     }
     
+    /**
+     *  @return Default Model text used for Observer updates (gets cached automatically)
+     */
+    public String cachedText() {
+        return null;
+    }
+    
 }
diff --git a/src/rails/game/state/Observable.java b/src/rails/game/state/Observable.java
index 0ac1ffb..9bdde04 100644
--- a/src/rails/game/state/Observable.java
+++ b/src/rails/game/state/Observable.java
@@ -15,11 +15,6 @@ public abstract class Observable implements Item {
     private final Item parent;
     private final Context context;
     
-    // stores observers and models if observable
-    // those are unobservable states themselves
-    private final HashSetState<Observer> observers;
-    private final HashSetState<Model> models;
-    
     /**
      * @param parent parent node in item hierarchy (cannot be null)
      * @param id id of the observable
@@ -43,45 +38,45 @@ public abstract class Observable implements Item {
 
         // if id is null this is an "unobservable" observable
         if (id == null) {
-            observers = null;
-            models = null;
         } else {
-            observers = HashSetState.create(this, null);
-            models = HashSetState.create(this, null);
             // add item to context if it has an id
             context.addItem(this);
         }
         
     }
+    
+    protected StateManager getStateManager() {
+        return context.getRoot().getStateManager();
+    }
 
     public void addObserver(Observer o) {
         checkState(id != null, "Cannot add observer to unobservable object");
-        observers.add(o);
+        getStateManager().addObserver(o, this);
     }
     
     public boolean removeObserver(Observer o) {
         checkState(id != null, "Cannot remove observer from unobservable object");
-        return observers.remove(o);
+        return getStateManager().removeObserver(o, this);
     }
     
     public ImmutableSet<Observer> getObservers() {
         checkState(id != null, "Cannot get observers of unobservable object");
-        return observers.view();
+        return getStateManager().getObservers(this);
     }
 
     public void addModel(Model m) {
         checkState(id != null, "Cannot add model to unobservable object");
-        models.add(m);
+        getStateManager().addModel(m, this);
     }
     
     public boolean removeModel(Model m) {
         checkState(id != null, "Cannot remove model from unobservable object");
-        return models.remove(m);
+        return getStateManager().removeModel(m, this);
     }
     
     public ImmutableSet<Model> getModels() {
         checkState(id != null, "Cannot get models of unobservable object");
-        return models.view();
+        return getStateManager().getModels(this);
     }
     
     /**
@@ -90,11 +85,15 @@ public abstract class Observable implements Item {
      */
     public void updateModels() {
         if (id == null) return;
-        for (Model m:models) {
+        for (Model m:this.getModels()) {
             m.update();
         }
     }
     
+    public boolean isObservable() {
+        return (id != null);
+    }
+    
     // Item methods
     
     public String getId() {
@@ -132,8 +131,8 @@ public abstract class Observable implements Item {
 
     
     /**
-     * @return text to be read by observers
+     * @return text delivered to observers, if no formatter is used
      */
-    public abstract String getText();
+    public abstract String observerText();
     
 }
diff --git a/src/rails/game/state/Observer.java b/src/rails/game/state/Observer.java
index 2cdd4ea..d392e41 100644
--- a/src/rails/game/state/Observer.java
+++ b/src/rails/game/state/Observer.java
@@ -5,6 +5,6 @@ package rails.game.state;
  */
 public interface Observer{
 
-    void update(Observable observable, String text);
+    void update(String text);
     
 }
diff --git a/src/rails/game/state/Root.java b/src/rails/game/state/Root.java
index 29c85e3..af481cd 100644
--- a/src/rails/game/state/Root.java
+++ b/src/rails/game/state/Root.java
@@ -9,27 +9,26 @@ public final class Root extends Context {
    public final static String ID = ""; 
    
    private StateManager stateManager;
-   private final HashMapState<String, Item> items = HashMapState.create(this, null);
+   private HashMapState<String, Item> items;
     
    private Root() {
+  
    }
 
    /**
     * @return a Root object with initialized StateManager embedded
     */
    public static Root create() {
+       // precise sequence to avoid any unintialized problems
        Root root = new Root();
        StateManager stateManager = StateManager.create(root, "states");
-       root.addStateManager(stateManager);
+       root.stateManager = stateManager;
+       root.items = HashMapState.create(root, null);
+       root.addItem(stateManager);
        root.addItem(root);
        return root;
    }
    
-   private void addStateManager(StateManager stateManager) {
-       this.stateManager = stateManager;
-       this.addItem(stateManager);
-   }
-   
    public StateManager getStateManager() {
        return stateManager;
    }
diff --git a/src/rails/game/state/State.java b/src/rails/game/state/State.java
index 59a95c0..324c99f 100644
--- a/src/rails/game/state/State.java
+++ b/src/rails/game/state/State.java
@@ -1,7 +1,5 @@
 package rails.game.state;
 
-import static com.google.common.base.Preconditions.*;
-
 /**
  * State is an abstract generic class
  * that defines the base layer of objects that contain game state.
@@ -15,45 +13,27 @@ import static com.google.common.base.Preconditions.*;
  */
 public abstract class State extends Observable {
     
-    // optional formatter
-    private Formatter<State> formatter = null;
-
     protected State(Item parent, String id) {
         super(parent, id);
         
-        
         // register if observable state
         if (id != null) {
             // check if parent is a model and add as dependent model
             if (parent instanceof Model) {
                 addModel((Model)parent);
             }
-            // check if there is a StateManager available
-            checkState(getContext().getRoot().getStateManager() != null, "Root of state has no StateManager attached");
-            // if so => register state there
-            getRoot().getStateManager().registerState(this);
+            this.getStateManager().registerState(this);
         }
     }
     
     // Observable methods
     /**
-     * For a state getText() it is identical to toString()
-     * If this does not work use a formatter
+     * {@inheritDoc}
+     * For a state defaultText() it is identical to toString()
+     * If this is not the correct behavior, overwrite it
      */
     @Override
-    public final String getText() {
-        if (formatter == null) {
-            return toString();
-        } else {
-            return formatter.formatValue(this);
-        }
-    }
-
-    /**
-     * Adds a Formatter 
-     * @param formatter
-     */
-    public void setFormatter(Formatter<State> formatter) {
-        this.formatter = formatter;
+    public String observerText() {
+        return toString();
     }
 }
\ No newline at end of file
diff --git a/src/rails/game/state/StateManager.java b/src/rails/game/state/StateManager.java
index 37cc852..3dd66eb 100644
--- a/src/rails/game/state/StateManager.java
+++ b/src/rails/game/state/StateManager.java
@@ -1,5 +1,7 @@
 package rails.game.state;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import java.util.List;
 import java.util.Set;
 
@@ -17,8 +19,16 @@ public final class StateManager extends Manager {
     protected static Logger log =
         LoggerFactory.getLogger(StateManager.class.getPackage().getName());
     
-    private final ChangeStack changeStack = ChangeStack.create();
+    private final ChangeStack changeStack = ChangeStack.create(this);
     private final HashSetState<State> allStates = HashSetState.create(this, null);
+
+    private final HashMultimapState<Observable, Observer> 
+        observers = HashMultimapState.create(this, null);
+    private final HashMapState<Observer, Formatter<? extends Observable>> 
+        formatters = HashMapState.create(this, null);
+    private final HashMultimapState<Observable, Model> 
+        models = HashMultimapState.create(this, null);
+    
     
 //  private final PortfolioManager portfolioManager = PortfolioManager.create(this, "portfolioManager");
 //  private final WalletManager walletManager = WalletManager.create(this, "walletManager");
@@ -58,6 +68,48 @@ public final class StateManager extends Manager {
     }
 
     /**
+     * Adds the combination of observer to observable
+     * @throws an IllegalArgumentException - if observer is already assigned to an observable
+     */
+    void addObserver(Observer observer, Observable observable) {
+        checkArgument(!observers.containsValue(observer), "Observer can only be assigned to one Observable");
+        observers.put(observable, observer);
+    }
+    
+    /**
+     * Adds the combination of observer to observable, using a Formatter
+     * @throws an IllegalArgumentException - if observer is already assigned to an observable
+     */
+    <T extends Observable>  void addObserver(Observer observer, Formatter<T> formatter) {
+        this.addObserver(observer, formatter.getObservable());
+        formatters.put(observer, formatter);
+    }
+    
+    boolean removeObserver(Observer observer, Observable observable) {
+        formatters.remove(observer);
+        return observers.remove(observable, observer);
+    }
+    
+    public ImmutableSet<Observer> getObservers(Observable observable) {
+        return observers.get(observable);
+    }
+    
+    /**
+     * Adds the combination of model to observable
+     */
+    void addModel(Model model, Observable observable) {
+        models.put(observable, model);
+    }
+
+    boolean removeModel(Model model, Observable observable) {
+        return models.remove(observable, model);
+    }
+    
+    public ImmutableSet<Model> getModels(Observable observable) {
+        return models.get(observable);
+    }
+    
+    /**
      * A set of states is given as input
      * and then calculates all observer to update in the correct sequence
      * 
@@ -121,7 +173,7 @@ public final class StateManager extends Manager {
      * @param states Set of states
      * @return all observers to be updated from states (either directly or via Models)
      */
-    Set<Observer> getObservers(Set<State> states){
+    private Set<Observer> getObservers(Set<State> states){
         
         Set<Observer> observers = Sets.newHashSet();
         
@@ -138,6 +190,20 @@ public final class StateManager extends Manager {
         return observers;
     }
     
+    void updateObservers(Set<State> states) {
+        for (Observable observable:getSortedObservables(states)) {
+            for (Observer observer:observable.getObservers()) {
+                // check if formatter is defined
+                if (formatters.containsKey(observer)) {
+                    observer.update(formatters.get(observer).observerText());
+                } else {
+                    // otherwise use observable text
+                    observer.update(observable.observerText());
+                }
+            }
+        }
+    }
+    
     /**
      * @param states Set of states
      * @return all models to be updated from states
diff --git a/src/rails/ui/swing/GridPanel.java b/src/rails/ui/swing/GridPanel.java
index f61dca3..5430170 100644
--- a/src/rails/ui/swing/GridPanel.java
+++ b/src/rails/ui/swing/GridPanel.java
@@ -180,7 +180,7 @@ implements ActionListener, KeyListener {
         }
 */
 
-        public void update(Observable observable, String text) {
+        public void update(String text) {
             // FIXME: There was a Boolean object submitted if the company is closed
             // TODO: Make this functionality available again
             // see above the old update method
diff --git a/src/rails/ui/swing/elements/Field.java b/src/rails/ui/swing/elements/Field.java
index 95dcaf9..9c440a8 100644
--- a/src/rails/ui/swing/elements/Field.java
+++ b/src/rails/ui/swing/elements/Field.java
@@ -87,7 +87,7 @@ public class Field extends JLabel implements Observer {
     @Override
     public void paintComponent(Graphics g) {
         if (observable != null && pull) {
-            setText(observable.getText());
+            setText(observable.observerText());
         }
         super.paintComponent(g);
     }
@@ -116,7 +116,7 @@ public class Field extends JLabel implements Observer {
     }
 
     /** Needed to satisfy the Observer interface. */
-    public void update(Observable observable, String text) {
+    public void update(String text) {
         setText(text);
     }
 
diff --git a/src/rails/ui/swing/elements/GUIStockSpace.java b/src/rails/ui/swing/elements/GUIStockSpace.java
index 0c388da..d1d0ab5 100644
--- a/src/rails/ui/swing/elements/GUIStockSpace.java
+++ b/src/rails/ui/swing/elements/GUIStockSpace.java
@@ -17,7 +17,6 @@ import org.slf4j.LoggerFactory;
 import rails.game.PublicCompany;
 import rails.game.StockSpace;
 import rails.game.state.Model;
-import rails.game.state.Observable;
 import rails.game.state.Observer;
 import rails.ui.swing.GUIToken;
 import rails.util.Util;
@@ -138,7 +137,7 @@ public class GUIStockSpace extends JLayeredPane implements Observer {
      * @see java.rails.util.Observer#update(java.rails.util.Observable,
      * java.lang.Object)
      */
-    public void update(Observable observable, String text) {
+    public void update(String text) {
         recreate();
     }
 
diff --git a/src/rails/ui/swing/hexmap/GUIHex.java b/src/rails/ui/swing/hexmap/GUIHex.java
index ad9d27d..8a36d78 100644
--- a/src/rails/ui/swing/hexmap/GUIHex.java
+++ b/src/rails/ui/swing/hexmap/GUIHex.java
@@ -37,7 +37,6 @@ import rails.game.Stop;
 import rails.game.Tile;
 import rails.game.TileOrientation;
 import rails.game.Token;
-import rails.game.state.Observable;
 import rails.game.state.Observer;
 import rails.ui.swing.GUIToken;
 import rails.util.Util;
@@ -897,6 +896,7 @@ public class GUIHex implements Observer {
     }
 
     // Used by Undo/Redo
+    // FIMXE: Does this still work?
     public void update(String notification) {
             // The below code so far only deals with tile lay undo/redo.
             // Tokens still to do
@@ -922,11 +922,4 @@ public class GUIHex implements Observer {
     public String toString () {
         return getName() + " (" + currentTile.getId() + ")";
     }
-
-    // FIMXE: Add the code here
-    public void update(Observable observable, String id) {
-        // TODO Auto-generated method stub
-        
-    }
-
 }

[Rails-commits] Branch 'rails2.0' - junit/rails src/rails

[Stefan F. <ste...@us...>]

 junit/rails/game/state/ModelImpl.java        |   22 ++-
 junit/rails/game/state/ModelTest.java        |   40 +++++
 junit/rails/game/state/StateImpl.java        |    6 
 junit/rails/game/state/StateManagerTest.java |  186 +++++++++++++++++++++++++++
 junit/rails/game/state/StateTest.java        |    8 -
 junit/rails/game/state/StateTestUtils.java   |    4 
 src/rails/game/state/StateManager.java       |  167 +++++++-----------------
 7 files changed, 305 insertions(+), 128 deletions(-)

New commits:
commit f4f887ca0517fe84536f6840c01efd5125093426
Author: Stefan Frey <ste...@we...>
Date:   Tue Jul 31 19:14:04 2012 +0200

    Added StateManagerTest and ModelTest
    Refactored StateManager topological sorting

diff --git a/junit/rails/game/state/ModelImpl.java b/junit/rails/game/state/ModelImpl.java
index e029de0..886f2e3 100644
--- a/junit/rails/game/state/ModelImpl.java
+++ b/junit/rails/game/state/ModelImpl.java
@@ -3,15 +3,27 @@ package rails.game.state;
 //An implementation only for testing
 class ModelImpl extends Model {
 
-    private final String text;
+    private final StringState text = StringState.create(this, "text");
     
-    ModelImpl(Item parent, String id, String text) {
+    private ModelImpl(Item parent, String id, String text) {
         super(parent, id);
-        this.text = text;
+        this.text.set(text);
     }
     
-    @Override
-    public String toString() {
+    static ModelImpl create(Item parent, String id, String text) {
+        return new ModelImpl(parent, id, text);
+    }
+    
+    void changeText(String text) {
+        this.text.set(text);
+    }
+    
+    State getState() {
         return text;
     }
+    
+    @Override
+    public String cachedText() {
+        return text.value();
+    }
 }
diff --git a/junit/rails/game/state/ModelTest.java b/junit/rails/game/state/ModelTest.java
new file mode 100644
index 0000000..ae16b1b
--- /dev/null
+++ b/junit/rails/game/state/ModelTest.java
@@ -0,0 +1,40 @@
+package rails.game.state;
+
+import static org.mockito.Mockito.*;
+import static org.junit.Assert.*;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ModelTest {
+
+    private static final String MODEL_ID = "Model";
+    private static final String MODEL_TEXT_INIT = "Init";
+    private static final String MODEL_TEXT_CHANGE = "Change";
+    
+    private Root root;
+    private ModelImpl model;
+    @Mock private Observer observer;
+    
+    @Before
+    public void setUp() {
+        root = StateTestUtils.setUpRoot();
+        model = ModelImpl.create(root, MODEL_ID, MODEL_TEXT_INIT);
+        StateTestUtils.startActionChangeSet(root);
+        model.addObserver(observer);
+    }
+    
+    @Test
+    public void testModel() {
+        assertEquals(MODEL_TEXT_INIT, model.observerText());
+        model.changeText(MODEL_TEXT_CHANGE);
+        StateTestUtils.close(root);
+        assertEquals(MODEL_TEXT_CHANGE, model.observerText());
+        verify(observer).update(MODEL_TEXT_CHANGE);
+    }
+
+}
diff --git a/junit/rails/game/state/StateImpl.java b/junit/rails/game/state/StateImpl.java
index 4d4e732..95cae54 100644
--- a/junit/rails/game/state/StateImpl.java
+++ b/junit/rails/game/state/StateImpl.java
@@ -5,11 +5,15 @@ class StateImpl extends State {
 
     private final String text;
     
-    StateImpl(Item parent, String id, String text) {
+    private StateImpl(Item parent, String id, String text) {
         super(parent, id);
         this.text = text;
     }
     
+    static StateImpl create(Item parent, String id, String text) {
+        return new StateImpl(parent, id, text);
+    }
+    
     @Override
     public String observerText() {
         return text;
diff --git a/junit/rails/game/state/StateManagerTest.java b/junit/rails/game/state/StateManagerTest.java
new file mode 100644
index 0000000..9816a5d
--- /dev/null
+++ b/junit/rails/game/state/StateManagerTest.java
@@ -0,0 +1,186 @@
+package rails.game.state;
+
+import static org.fest.assertions.api.Assertions.assertThat;
+import static org.fest.assertions.api.Assertions.failBecauseExceptionWasNotThrown;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+
+@RunWith(MockitoJUnitRunner.class)
+public class StateManagerTest {
+
+    private final static List<String> ID = 
+            ImmutableList.of("A1" , "A2" , "A3", "B1", "B2", "C1", "C2", "C3", "D", "E", "F");
+    
+    private Root root;
+    private StateManager sm;
+    
+    @Mock private State state;
+    @Mock private Observer observer;
+    @Mock private Model model; 
+    private ModelImpl m_A1, m_A2, m_A3, m_B1, m_B2, m_C1, m_C2, m_C3, m_D, m_E, m_F;
+    @Mock private Observer o_A1, o_A2, o_A3, o_B1, o_B2, o_C1, o_C2, o_C3;
+    
+    @Before
+    public void setUp() {
+        root = StateTestUtils.setUpRoot();
+        sm = root.getStateManager();
+        
+        // initialize the models
+        m_A1 = ModelImpl.create(root, ID.get(0), ID.get(0));
+        m_A2 = ModelImpl.create(root, ID.get(1), ID.get(1));
+        m_A3 = ModelImpl.create(root, ID.get(2), ID.get(2));
+        m_B1 = ModelImpl.create(root, ID.get(3), ID.get(3));
+        m_B2 = ModelImpl.create(root, ID.get(4), ID.get(4));
+        m_C1 = ModelImpl.create(root, ID.get(5), ID.get(5));
+        m_C2 = ModelImpl.create(root, ID.get(6), ID.get(6));
+        m_C3 = ModelImpl.create(root, ID.get(7), ID.get(7));
+        m_D = ModelImpl.create(root, ID.get(8), ID.get(8));
+        m_E = ModelImpl.create(root, ID.get(9), ID.get(9));
+        m_F = ModelImpl.create(root, ID.get(10), ID.get(10));
+        
+        // define connections
+        m_A1.addModel(m_B1);
+        m_A1.addModel(m_B2);
+        m_A1.addModel(m_C2);
+        m_A2.addModel(m_B2);
+        m_A3.addModel(m_C3);
+        m_B1.addModel(m_C1);
+        m_B2.addModel(m_C2);
+        
+        // D is special that it depends on the state in A3 directly
+        m_A3.getState().addModel(m_D);
+        m_D.addModel(m_A3);
+        
+        // E and F form a cycle
+        m_E.addModel(m_F);
+        m_F.addModel(m_E);
+        
+        // observers
+        m_A1.addObserver(o_A1);
+        m_A2.addObserver(o_A2);
+        m_A3.addObserver(o_A3);
+        m_B1.addObserver(o_B1);
+        m_B2.addObserver(o_B2);
+        m_C1.addObserver(o_C1);
+        m_C2.addObserver(o_C2);
+        m_C3.addObserver(o_C3);
+  }
+
+    @Test
+    public void testRegisterState() {
+        sm.registerState(state);
+        assertThat(sm.getAllStates()).contains(state);
+        sm.deRegisterState(state);
+        assertThat(sm.getAllStates()).doesNotContain(state);
+    }
+
+    @Test
+    public void testAddObserver() {
+        sm.addObserver(observer, state);
+        assertThat(sm.getObservers(state)).contains(observer);
+        sm.removeObserver(observer, state);
+        assertThat(sm.getObservers(state)).doesNotContain(observer);
+    }
+
+    @Test
+    public void testAddModel() {
+        sm.addModel(model, state);
+        assertThat(sm.getModels(state)).contains(model);
+        sm.removeModel(model, state);
+        assertThat(sm.getModels(state)).doesNotContain(model);
+    }
+    
+    private void checkOrderings(List<Model> updates) {
+        for (Model m:updates) {
+            for (Model dep_m: m.getModels()) {
+                assertThat(updates.indexOf(dep_m)).isGreaterThan(updates.indexOf(m));
+            }
+        }
+    }
+    
+
+    private void assertObservables(List<? extends Model> expected, Set<ModelImpl> updated) {
+        // get all embedded states that are included
+        Set<State> states = Sets.newHashSet();
+        for (ModelImpl m:updated) {
+            states.add(m.getState());
+        }
+        // get all observables that are updated
+        List<Model> toUpdate = sm.getModelsToUpdate(states);
+        // check that all non-states observables are the updated models and the expected models
+        assertThat(toUpdate).containsAll(expected);
+        // ... and does not have duplicates
+        assertThat(toUpdate).doesNotHaveDuplicates();
+        // ... and has the same size
+        assertEquals(expected.size(), toUpdate.size());
+        // and check ordering
+        checkOrderings(toUpdate);
+   }
+    
+    @Test
+    public void testObservablesToUpdate() {
+        // nothing <= nothing
+        assertObservables(ImmutableList.<Model>of(),ImmutableSet.<ModelImpl>of());
+        // A1, B1, B2, C1, C2 <= A1
+        assertObservables(ImmutableList.of(m_A1, m_B1, m_B2, m_C1, m_C2),ImmutableSet.of(m_A1));
+        // A2, B2, C2 <= A2
+        assertObservables(ImmutableList.of(m_A2, m_B2, m_C2),ImmutableSet.of(m_A2));
+        // A3, C3 <= A3
+        assertObservables(ImmutableList.of(m_A3, m_C3, m_D),ImmutableSet.of(m_A3));
+        // B1, C1 <= B1
+        assertObservables(ImmutableList.of(m_B1, m_C1),ImmutableSet.of(m_B1));
+        // B2, C2 <= B2
+        assertObservables(ImmutableList.of(m_B2, m_C2),ImmutableSet.of(m_B2));
+        // C1, C2 <= C1, C2
+        assertObservables(ImmutableList.of(m_C1, m_C2),ImmutableSet.of(m_C1, m_C2));
+        // Combinations:
+        // A1, A2, B1, B2, C1, C2 <= A1, A2
+        assertObservables(ImmutableList.of(m_A1, m_A2, m_B1, m_B2, m_C1, m_C2),ImmutableSet.of(m_A1, m_A2));
+        // A1, A2, A3, B1, B2, C1, C2, C3 <= A1, A2, A3
+        assertObservables(ImmutableList.of(m_A1, m_A2, m_A3, m_B1, m_B2, m_C1, m_C2, m_C3, m_D),ImmutableSet.of(m_A1, m_A2, m_A3));
+        // A2, B1, B2, C1, C2 <= A2, B1
+        assertObservables(ImmutableList.of(m_A2, m_B1, m_B2, m_C1, m_C2),ImmutableSet.of(m_A2, m_B1));
+        
+        try{
+            assertObservables(ImmutableList.<Model>of(), ImmutableSet.of(m_E));
+            failBecauseExceptionWasNotThrown(IllegalStateException.class);
+          } catch (Exception e) {
+            assertThat(e).isInstanceOf(IllegalStateException.class);
+          }
+    }
+
+    @Test
+    public void testUpdateObservers() {
+        sm.updateObservers(ImmutableSet.of(m_A1.getState()));
+        verify(o_A1).update(ID.get(0));
+        verify(o_B1).update(ID.get(3));
+        verify(o_B2).update(ID.get(4));
+        verify(o_C1).update(ID.get(5));
+        verify(o_C2).update(ID.get(6));
+        verifyZeroInteractions(o_A2, o_A3, o_C3);
+    }
+
+    @Test
+    public void testGetChangeStack() {
+        assertNotNull(sm.getChangeStack());
+    }
+
+    @Test
+    public void testGetPortfolioManager() {
+        assertNotNull(sm.getPortfolioManager());
+    }
+
+}
diff --git a/junit/rails/game/state/StateTest.java b/junit/rails/game/state/StateTest.java
index dcc2bef..0b2dc1d 100644
--- a/junit/rails/game/state/StateTest.java
+++ b/junit/rails/game/state/StateTest.java
@@ -21,10 +21,10 @@ public class StateTest {
     @Before
     public void setUp() {
         root = Root.create();
-        state = new StateImpl(root, STATE_ID, null);
-        model = new ModelImpl(root, MODEL_ID, null);
-        state_model = new StateImpl(model, STATE_ID, null);
-        state_wo_id = new StateImpl(model, null, STATE_TEXT);
+        state = StateImpl.create(root, STATE_ID, null);
+        model = ModelImpl.create(root, MODEL_ID, null);
+        state_model = StateImpl.create(model, STATE_ID, null);
+        state_wo_id = StateImpl.create(model, null, STATE_TEXT);
     }
     
     @Test
diff --git a/junit/rails/game/state/StateTestUtils.java b/junit/rails/game/state/StateTestUtils.java
index b47b9ac..b9b580c 100644
--- a/junit/rails/game/state/StateTestUtils.java
+++ b/junit/rails/game/state/StateTestUtils.java
@@ -40,6 +40,10 @@ class StateTestUtils {
         root.getStateManager().getChangeStack().redo();
     }
     
+    public static ChangeSet getCurrentChangeSet(Root root) {
+        return root.getStateManager().getChangeStack().getCurrentChangeSet();
+    }
+    
     public static ChangeSet getLastClosedChangeSet(Root root) {
         return root.getStateManager().getChangeStack().getLastClosedChangeSet();
         
diff --git a/src/rails/game/state/StateManager.java b/src/rails/game/state/StateManager.java
index 26b9187..0b95760 100644
--- a/src/rails/game/state/StateManager.java
+++ b/src/rails/game/state/StateManager.java
@@ -2,15 +2,20 @@ package rails.game.state;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
-import java.util.List;
+import java.util.Collection;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Queue;
 import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.collect.HashMultimap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
 
@@ -39,30 +44,21 @@ public final class StateManager extends Manager implements DelayedItem {
         return new StateManager(parent, id);
     }
     /**
-     * Register states 
-     * Remark: Portfolios and Wallets get added from their respective managers automatically
+     * Register states (usually done automatically at state creation)
      */
     void registerState(State state) {
         allStates.add(state);
-//        if (state instanceof Portfolio) {
-//            return portfolioManager.addPortfolio((Portfolio<?>) state);
-//        } else if (state instanceof Wallet) {
-//            return walletManager.addWallet((Wallet<?>) state);
-//        }
     }
     
     /**
-     * De-Register states 
-     * Remark: Portfolios and Wallets are removed from their respective managers automatically
+     * De-Register states
      */
     boolean deRegisterState(State state) {
-        if (!allStates.remove(state)) return false;
-//        if (state instanceof PortfolioMap) {
-//            return portfolioManager.removePortfolio((PortfolioMap<?>) state);
-//        } else if (state instanceof Wallet) {
-//            return walletManager.removeWallet((Wallet<?>) state);
-//        }
-        return true;
+        return allStates.remove(state);
+    }
+    
+    ImmutableSet<State> getAllStates() {
+        return allStates.view();
     }
 
     /**
@@ -84,6 +80,8 @@ public final class StateManager extends Manager implements DelayedItem {
     
     /**
      * Adds the combination of model to observable
+     * @param Model the model that tracks the observable
+     * @param Observable the observable to monitor
      */
     void addModel(Model model, Observable observable) {
         models.put(observable, model);
@@ -96,128 +94,61 @@ public final class StateManager extends Manager implements DelayedItem {
     public ImmutableSet<Model> getModels(Observable observable) {
         return models.get(observable);
     }
-    
+
     /**
      * A set of states is given as input
      * and then calculates all observer to update in the correct sequence
      * 
-     * It uses a topological sort algorithm (Kahn 1962)
+     * It uses a topological sort based on DFS
      * 
-     * @param states Set of states
-     * @return sorted list of all observables (states and models)
+     * @param states that have changed
+     * @return sorted list of all models to be updated
      */
-    List<Observable> getSortedObservables(Set<State> states) {
-
-        // 1: define all models
-        Set<Model> models = getModels(states);
-        
-        // 2: define graph
-        Multimap<Model, Observable> edges = HashMultimap.create(); 
-        
-        // 2a: add edges that start from states
-        for (State s:states) {
-            for (Model m:s.getModels()) {
-                edges.put(m, s);
-            }
-        }
-        
-        // 2b: add edges that start from models
-        for (Model m1:models) {
-            for (Model m2:m1.getModels()) {
-                edges.put(m2, m1);
-            }
-        }
-
-        // 3: run topological sort
-        List<Observable> sortedList = Lists.newArrayList();
-        List<Observable> startNodes = Lists.newArrayList();
-        startNodes.addAll(states);
-        
-        while (!startNodes.isEmpty()) {
-            // remove node n
-            Observable n = startNodes.remove(0);
-            // insert node into sortedList 
-            sortedList.add(n);
-            for (Model m:n.getModels()) {
-                edges.remove(m, n);
-                // check if m is now a start node
-                if (!edges.containsKey(m)) {
-                    startNodes.add(m);
-                }
-            }
-        }
+    private static enum Color {WHITE, GREY, BLACK}
+    ImmutableList<Model> getModelsToUpdate(Collection<State> states) {
+        // Topological sort
+        // Initialize (we do not use WHITE explicitly, but implicit)
+        Map<Observable, Color> colors = Maps.newHashMap();
+        LinkedList<Model> topoList = Lists.newLinkedList();
         
-        // if graph is not empty => cyclical graph
-        if (!edges.isEmpty()) {
-            log.debug("StateManager: Cyclical graph detected in State/Model relations.");
-            // add remaining models to the end
-            sortedList.addAll(edges.keySet());
+        // For all states
+        for (State s: states) {
+            topoSort(s, colors, topoList);
         }
-        
-        return sortedList;
+        log.debug("Observables to Update = " + topoList.toString());
+        return ImmutableList.copyOf(topoList);
     }
     
-    /**
-     * @param states Set of states
-     * @return all observers to be updated from states (either directly or via Models)
-     */
-    private Set<Observer> getObservers(Set<State> states){
-        
-        Set<Observer> observers = Sets.newHashSet();
-        
-        // all direct observers
-        for (State s:states){
-            observers.addAll(s.getObservers());
-        }
-        
-        // all indirect observers
-        for (Model m:getModels(states)){
-            observers.addAll(m.getObservers());
+    private void topoSort(Observable v, Map<Observable, Color> colors, LinkedList<Model> topoList) {
+        colors.put(v, Color.GREY);
+        for (Model m:v.getModels()) {
+            if (!colors.containsKey(m)) {
+                topoSort(m, colors, topoList);
+            } else if (colors.get(m) == Color.GREY) {
+                throw new IllegalStateException("Graph of Observables contains Cycle");
+            }
         }
-        
-        return observers;
+        colors.put(v, Color.BLACK);
+        if (v instanceof Model) topoList.addFirst((Model)v);
     }
     
+    
     void updateObservers(Set<State> states) {
-        for (Observable observable:getSortedObservables(states)) {
-            for (Observer observer:observable.getObservers()) {
-                    observer.update(observable.observerText());
+        // all direct observers
+        for (State s:states){
+            for (Observer o:s.getObservers()) {
+                o.update(s.observerText());
             }
         }
-    }
-    
-    /**
-     * @param states Set of states
-     * @return all models to be updated from states
-     */
-    Set<Model> getModels(Set<State> states) {
-        
-        Set<Model> allModels = Sets.newHashSet();
-        
-        // add all models updated from states directly
-        for (State s:states) {
-            allModels.addAll(s.getModels());
-        }
         
-        // then add models called indirectly
-        ImmutableSet<Model> checkModels = ImmutableSet.copyOf(allModels);
-        Set<Model> newModels = Sets.newHashSet();
-        while (!checkModels.isEmpty()) {
-            for (Model m1:checkModels) {
-                for (Model m2:m1.getModels()) {
-                    if (!allModels.contains(m2)) {
-                        allModels.add(m2);
-                        newModels.add(m2);
-                    }
-                }
+        // all indirect observers
+        for (Model m:getModelsToUpdate(states)) {
+            for (Observer o:m.getObservers()) {
+                o.update(m.observerText());
             }
-            checkModels = ImmutableSet.copyOf(newModels);
-            newModels.clear();
         }
-        return allModels;
     }
     
-    
 //    void registerReceiver(Triggerable receiver, State toState) {
 //    }