[josm-dev] Jumbo Patch

[Gabriel Ebner]

On Sun, Dec 16, 2007 at 10:13:47PM +0100, Petr Nejedly wrote:
> I was just trying to be nice to JOSM and plugin developers, but you're basically
> saying that it is OK to rewrite the core heavily and break plugins if it serves
> the purpose. Why I don't like such an approach, JOSM is not my project and
> I'm only trying to improve it to better cover my needs.

It's not like we're breaking API compatibility every day. :-)

AFAIK the last time we broke some plugins was with the API 0.5 changes.  And I
don't know how we could have prevented that.  If a plugins wants to add a
segment, but neither JOSM nor the server use them anymore, there simply isn't
a way to make this work easily.

Of course, we could have provided transition code, but it would take quite an
elaborate abstraction to conceal the differences.  And that takes time and
effort.

Unfortunately pretty much all of the code that operates on the data set makes
intricate assumptions about the inner workings, even simple changes like
adding a node to a way make delicate assumptions about the way JOSM undos them.

And these assumptions cannot be abstracted away by making all instance
variables private.  You still have to make the assumption that you change a
way by first cloning it, then modifying it and then adding a ChangeCommand to
the undo list.  And this precise assumption is one that is broken by the patch
in question.

Of course, we could provide common commands directly without requiring the
code to actually delve into the gory details itself, but that's currently not
easily possible.

As it stands, we cannot combine two commands properly, which makes it nearly
impossible to build larger commands from smaller ones -- if you want to
generate a sufficiently complex command, you'll have to do all sorts of
low-level work.  The only sensible way to combine two commands would be to
commit one to the undo list and then the other other -- but that would require
the user to press undo twice.  If we wouldn't commit the first one first,
changes to the same primitives in the second command would overwrite the ones
from the first.

Of course I'd like the code to be as independent as possible from the concrete
implementation, but that isn't a simple matter of wrapping all accesses to a
public instance variable in getters and setters.

> So far all the changes I have in my sleeve (already coded) are nonintrusive,
> but I think of few optimizations that would need update of unrelated parts,
> and providing real scalability into JOSM would certainly require heavy
> restructuralization, including wrapping some fields or some other measure
> to cover external changes.

JOSM wasn't designed to work on large data sets.  It even stores the
primitives in a linked list.  No index on the id.  No index on the coordinate.

Obviously it will take big changes to make it even remotely scalable, but
these changes are not something to cover.  Plugins (especially the validator
plugin) should have access to them too.

> I just want to know whether such changes will be accepted.

This particular patch has only generated such a heated discussion because it
was so intrusive.  Nonintrusive patches have always been accepted so far,
especially if they improve the performance.

  Gabriel.