Re: [Rails-devel] Some (multi-)undo related bugs

[Erik V. <eri...@xs...>]

Stefan,

Since you created GenericState<>, I realized that State is redundant. My
thought was (and is) that we should try to merge State and GenericState<>
into State<>. 

That would make IntegerState etc. basically redundant, as it could be
replaced by State<Integer> etc.
Alternatively, if we would need the extra methods, we can also have
subclasses like
	class IntegerState extends State<Integer> {...}
where we can for instance add an add() method, and still reuse the set() and
get() methods with automatic casting. No more need for intValue(). 

On first sight, such an approach would more appeal to me than yours, but I
must admit that I have not worked it out beyond testing that the above
subclassing approach works (to my surprise, I must confess). 

In any case, as State already extends ModelObject, we get that superclass
for free.
On IntegerChange etc., I don't see the added value of these new moves, if we
already have ObjectChange, but perhaps I'm dumb...

Here is the full code of my test classes:

package rails.game.state;
public class IntegerState2 extends GenericState<Integer> {
    public IntegerState2 (String name, int i) {
        super (name, i);
    }
    public IntegerState2 (String name) {
        super (name, 0);
    }    
    public void add (int i) {
        set (get()+i);
    }
}	

package test;
import rails.game.state.IntegerState2;
public class Test {
    public static void main(String[] args) {
        IntegerState2 i = new IntegerState2 ("I", 1);
        System.out.println(i.get());
        i.set(3);
        System.out.println(i.get());
        i.add(2);
        System.out.println(i.get());    
    }
}

which prints
1
3
5

Looks pretty lean to me...

I'm not sure if my idea could be extended to ArrayListState<E> extends
State<List<E>>, but perhaps it's worth a try...

Erik.


-----Original Message-----
From: Stefan Frey [mailto:ste...@we...] 
Sent: Wednesday 25 August 2010 23:43
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] Some (multi-)undo related bugs

Erik:
other  implementation details follow here as they arose from to the topic in

the subject:

I have added two new state classes: ArrayListState and HashMapState, that 
create ModelObjects, which use the already defined Move classes for changes
of 
ArrayLists and HashMaps. It basically wraps the ModelObject/State around 
standard ArrayLists and HashMaps and uses the standard method names to 
manipulate them. In principle it would be possible to fully implement the 
Map/List interfaces, but I was too lazy to write all required methods for 
that.

My reason for adding those is that at least for the approach above protects 
against accidentally using the non-move methods on list or maps, which are
in 
fact stateful and thus require the moves instead. And I still prefer using
the 
standard syntax stateList.add(object) instead of new AddToList("NameOfList",

stateList, object). And it allows to create more complicated operations
using 
the atomic moves (for example addAll or initFromMap). It is possible to 
retrieve the wrapped list/map as an unmodifiable view.

This allowed fixing the opertingCompanies bug in 1856 and the tilesPerColour

problem in 18EU.

Currently they do not inherit from neither ModelObject/State as the current 
usage has not required them to do so. But it is possible to do so, but I
would 
prefer to inherit from ModelObject directly. 

If you do not mind I would like to add an according HashSetState class and  
replace BooleanState, IntegerState and StringState with more specialized 
versions that inherit from ModelObject instead of State and have according 
simple Moves (BooleanChange, IntegerChange, StringChange) and it would allow

to introduce an easier syntax, for example for boolean: booleanVar.set(true)

or booleanVar.set(false) and booleanVar.is() or booleanVar.isNot()

This would also allow to deprecate (and remove later) the State class,as
there 
is already a replacement available with GenericState. All those changes make
the StateI interface unnecessary. It could still be used as a mere marker 
interface or have the getName() function at most.
Stefan