Menu
▾
▴
Re: [Rails-devel] Some (multi-)undo related bugs
|
From: Stefan F. <ste...@we...> - 2010-08-31 22:41:59
|
Erik,
I think you convinced me that there is no need to have any state classes,
which do not subclass ModelObject. The increased complexity of a parallel
structure clearly outweighs any performance gain.
I would even go one step further: The supposed difference between state and
model classes is unnecessary and I would rename all state classes to model
classes: Thus instead of IntegerState we would have IntegerModel and so on.
If we still have different packages, I would separate GenericModels (as e.g.
IntegerModel) from RailsModels (as BaseTokensModel).
I only differ in the implementation details:
I would prefer to derive the specific models (as ArrayListState/Model
IntegerState/Model) directly from ModelObject, as the GenericState/Model
mechanism creates a new object for each change (move) of the model. My
approach would reuse the existing object and the move would contain only the
delta. (or for the IntegerModel: it allows to use int fields instead of
Integer ones for the move and model objects).
Stefan
PS: In addition you convinced me that any trigger approach should work on the
model/state objects directly: It should allow to build conditions in the xml
files on defined state changes.
(Hypothetical) Example for 1835 BA certificates:
<Certificates>
<Available>
<Trigger="BY_fullysold" value="yes">
<Trigger="SX_fullysold" value="yes" operator="and">
</Available>
</Certificates>
On Thursday, August 26, 2010 10:37:20 pm Erik Vos wrote:
> 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
>
>
>
> ---------------------------------------------------------------------------
> --- Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
> Be part of this innovative community and reach millions of netbook users
> worldwide. Take advantage of special opportunities to increase revenue and
> speed time-to-market. Join now, and jumpstart your future.
> http://p.sf.net/sfu/intel-atom-d2d
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
|
