[PATCH] OsmPrimite shouldn't expose public fields

[jttt]

Currently OsmPrimitive and its descendants (Node, Way and Relation) expose public fields which makes it hard to implement caching or spatial index to speed up rendering.

It should be changed so that internal state of OsmPrimitive is modifiable only using methods or at least accessors.

[jttt]

Patch to replace timestamp and parsedTimestap with setter/getter

[jttt]

Patch to change hasDirectionKeys and isTagged to read only properties

[jttt]

Patch to replace hasDirectionKeys and isTagged with readonly properties (in plugins)

[jttt]

If patches are applied I will prepare another patches for rest of public fields

[stoecker]

A short review:

The plugins_tagged.patch for Validator fails. These two calls are there to ensure the two variables containing the uninteresting/direction keys are initialized. If you remove them, we need another method to ensure this.

[jttt]

I've removed that two variable completely. isTagged/hasDirectionTags value is calculated every time that methods are called. Caching will return after keys are made private so OsmPrimitive knows that cache has to be invalidated.

[anonymous]

You misunderstood. Validator does not use these variables, but the two string lists pointing to the keys. They have been initialized using the two functions you removed.

[jttt]

I see I didn't notice that. New patch simply adds getter for uninteresting/directionKeys that make sure the lists are initialized.

[stoecker]

They will kill me after this again, as it will break plugins. Phew.
Checkin begin of next week I would say.

[anonymous]

Yes, plugins that are not in osm repository will not work. Maybe it should be all changed in one go so plugins will break only once. Or keep public fields for now, only make them deprecated?

[stoecker]

In r1499.

[jttt]

Here is another patch, this time for eastNorth and coor on Node. The patch provides setCoor and setEastNorth setters and makes sure eastNorth and coor are kept in sync.

Getters should be provided as well but it's one click job in Eclipse to use getCoor in all places instead of direct access to the coor field. So I guess it would be easier to do refactoring directly instead of reviewing the patch.

Fields coor and eastNorth should be kept public and plugins should not switch to setters and getters until this gets into josm-tested.jar. This way no api breakage should occur.

When coor becames private then it might be worthy to replace LatLon instance with two doubles and create new LatLon instance in getCoor method. That would save about 20 bytes per node on 32b machine, that's 5% of JOSM memory usage.

[stoecker]

In r1636.

Pleas open a new report next time. Thanks.

[jttt]

Can you please refactor to getCoor/getEastNorth as well? Either using refactoring in IDE or using the getcoor.patch. I was hoping to encalupse latLon later to save some more memory.

Btw. JOSM memory usage is not that bad now. I've managed to open 830MB osm file (using 1700MB heap memory). If mappaint was a bit faster then JOSM might became good tool for mass edits.

[stoecker]

In r1640.

I do not use eclipse, so such refactorings must be supplied as patches :-)