[josm-dev] Jumbo Patch

Brent Easton b.easton at exemail.com.au
Sun Dec 16 23:17:35 GMT 2007


Hi Gabriel,

Just a few thoughts below.

Regards,
Brent.

*********** REPLY SEPARATOR  ***********

On 16/12/2007 at 11:27 PM Gabriel Ebner  wrote:

>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. 

I believe it happens more regularly than that.

>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.


Well, there is. If the segments where being added via a standard call, that call could have been just changed to do nothing.


>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.


No, it would have been pretty easy actually as per previous note.


>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.


That's where the mistake lies.


>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.


Sometimes, the time comes in a project when it becomes obvious that something is deeply wrong with the core data structures and processing. You have two choices

1. Step back, take time, analyze, examine all the lessons learned, review the existing and future needs, then plan carefully and do a major restructure. Yes, it is not 'easy', but when it is done, it becomes easy to move forward quickly.

2. Keep patching around the edges, becoming increasingly frustrated with the impossiblity of adding needed features.



>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.


No, I don't think anyone is suggesting that adding a bunch of getter/setter's is going to fix the flaws in the current structure. I don't think that is what this discussion is about. Getters/setter is one tool in the toolbox for building a more maintainable, less bug-ridden structure. 

Someone claimed that adding setter/getters would add hundred's of lines to their source files. If you have to add that many, then the code is organised wrongly. The 'intelligence' needs to bought down into the lower level objects.

I have reached situations like this many times. My gut feeling is that the JOSM core needs to be redesigned. Conceptually, in the greater scheme of things, what JOSM does is not that complex and it should be possible to come up with a highly optimized, extensible core that does everything needed. If this where to go ahead, then it would make sense to design this with OO principles in mind to make it maintainable. If a new plugin or piece of code needs to hook into something in the core in a non-standard way, then we add the appropriate hooks into the core to allow this to happen.



>> 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.

____________________________________________________________
Brent Easton                       
Analyst/Programmer                               
University of Western Sydney                                   
Email: b.easton at uws.edu.au





More information about the josm-dev mailing list