GeoTools

Make GeoTools Style interfaces mutable

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.6-M3
  • Fix Version/s: 2.6-M3
  • Component/s: main
  • Labels:
    None

Description

We have introduced many new methods on the journey to symbology encoding 1.1 support:

  • getName
  • getDescrption

We need to ensure that each one of these is mutable:

  • setName( String )
  • setDescription( Description )

One implementation detail encountered while fixing this bug is how to "cast" from an org.opengis.style.Description to a org.geotools.styling.Description that has set methods.

It was a choice between:

  • DescriptionImpl.cast( org.opengis.style.Description ); or
  • new DescriptionImpl( org.opengis.style.Description )

I have gone with the initial approach of a cast method - so that I can perform an instanceof check and avoid doing any extra creation.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jody Garnett added a comment -
Attach initial patch; still missing factory methods to create this stuff directly - but at least now setter methods are available.
Show
Jody Garnett added a comment - Attach initial patch; still missing factory methods to create this stuff directly - but at least now setter methods are available.
Hide
Permalink
Andrea Aime added a comment -
Hi Jody, the patch looks good.
I remember we have a similar casting methods on ReferencedEnvelope, but there they are called "references".. pity, we could have estabilshed some conventions for naming methods like that.

Most cast() method do not have a javadoc, I think it's necessary otherwise people will start wondering what the cast method is about (how different it is from a programming language cast).
On the other side xxxImpl are not supposed to be used directly by people, but then again we don't have a convention to make that explicit.
Show
Andrea Aime added a comment - Hi Jody, the patch looks good. I remember we have a similar casting methods on ReferencedEnvelope, but there they are called "references".. pity, we could have estabilshed some conventions for naming methods like that. Most cast() method do not have a javadoc, I think it's necessary otherwise people will start wondering what the cast method is about (how different it is from a programming language cast). On the other side xxxImpl are not supposed to be used directly by people, but then again we don't have a convention to make that explicit.
Hide
Permalink
Jody Garnett added a comment -
Thanks for the review andea; I have confirmed that all tests pass. I am actually thinking I may be able to get away with the "cast" methods being package visible... I am not really thrilled to be breaking out new api.

Would you like me to rename the method? I always figured ReferencedEnvelope.reference was named based on the fact that it was a "Referenced"Enveloped we were adapting to?
Show
Jody Garnett added a comment - Thanks for the review andea; I have confirmed that all tests pass. I am actually thinking I may be able to get away with the "cast" methods being package visible... I am not really thrilled to be breaking out new api. Would you like me to rename the method? I always figured ReferencedEnvelope.reference was named based on the fact that it was a "Referenced"Enveloped we were adapting to?
Hide
Permalink
Andrea Aime added a comment -
Keeping it package level sounds good. No need to rename methods.
Show
Andrea Aime added a comment - Keeping it package level sounds good. No need to rename methods.
Hide
Permalink
Jody Garnett added a comment -
reduced the cast methods to package level - they are used by the factory a bit in order to manipulate data as well.
-r 33813 on trunk
Show
Jody Garnett added a comment - reduced the cast methods to package level - they are used by the factory a bit in order to manipulate data as well. -r 33813 on trunk

People

  • Assignee:
    Unassigned
    Reporter:
    Jody Garnett
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: