On Fri, Dec 21, 2007 at 12:06:14PM -0800, Dave Hansen wrote: > I think ChangeCommand sucks (and by implication, the wnew approach).
ChangeCommand has one nice thing going for it, it's just one single command. Anything else would require at the very least 4 commands (names are just examples): - ChangeTagsCommand - ChangeCoordinatesCommand - ReplaceWayNodesCommand - ReplaceRelationMembersCommand And I guess this is too coarse too. So probably more are needed. And hitting undo will wreak havoc if there's just a single bug in these commands. The commands have historically been a very stable piece of code. ChangeCommand's last significant change took place in April 2006. > It's way too coarse, and I'd like to make it basically invalid to > perform on ways unless it's applied atomically with its creation. Did I miss anything in the patches? You still gather all the commands up in a SequenceCommand and then apply them long after you've created them. You don't apply them atomically with their creation, but rather apply more fine-grained modifications when they're executed. I had suggested applying the commands immediately upon creation somewhere in the thread, so Main.ds would actually represent the current state of the world, but it didn't seem to have caught on. > Ways just have too much state, and can get too easily made inconsistent. Groping in the dark for the light switch is still groping in the dark even if you can only turn the switch one way. You can still have two commands canceling each other out when performed in one order and reinforcing themselves in another order. You won't get around knowing the current state of the data set when creating a command. Creating a way and adding a node to it works only in one order, not in the other. > As for leaking references, having any kind of reverse lookup system will > make them more likely. But, those are bugs that I am very willing to > fix. I was not talking about more likely. I was talking about definite. E.g. you need to remove the back-references when deleting a way, else the back-references will not only be wrong, they will also prevent the way from being garbage collected, creating a leak. The same when undoing the addition of a way. Not to mention that this will need all sorts of hairy dispose() calls (or clearAllNodes() if you prefer) at least in MergeVisitor. This is one of the places were changes don't go through the undoRedo list, but are applied directly. (And no, making the changes through the undoRedo list is not an option, as that would set the modified bits and make JOSM upload all the merged primitives.) What's wrong with a good old hash table in DataSet that you can remove or rebuild when necessary? Gabriel. _______________________________________________ josm-dev mailing list [email protected] http://lists.openstreetmap.org/cgi-bin/mailman/listinfo/josm-dev
