[josm-dev] Jumbo Patch

Dave Hansen dave at sr71.net
Fri Dec 21 20:06:14 GMT 2007


On Sun, 2007-12-16 at 19:54 +0100, Gabriel Ebner wrote:
> E.g. until now, this was the common way to change the node list of a way
> (which you'd use if you want to split a way, combine ways, simplify a way,
> ...):
> 
>   Way wnew = new Way(w);
>   wnew.nodes.clear();
>   wnew.nodes.addAll(newNodes);
>   Main.main.undoRedo.add(new ChangeCommand(w, wnew));
> 
> But the following was also legal:
> 
>   Way wnew = new Way(w);
>   wnew.nodes.clear();
>   wnew.nodes.addAll(newNodes);
> 
>   if (!askUserIfOk(wnew)) {
>     return;
>   } 

I know what I want is a pretty big change in behavior here.

I think ChangeCommand sucks (and by implication, the wnew approach).
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.  Ways
just have too much state, and can get too easily made inconsistent.
With my patches, there are precious few of them left.

So, I've replaced virtually all of the ChangeCommands with the commands
that we talked about, like ReplaceNodeInWay, AddNodeToWay and
RemoveNodeFromWay.  They can also very nicely undo themselves without
having to rely on the Command class's foo.cloneFrom(oldFoo).  

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.  

-- Dave





More information about the josm-dev mailing list