GeoTools
  1. GeoTools
  2. GEOT-2684

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

        Hide
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: