Commons Configuration
  1. Commons Configuration
  2. CONFIGURATION-205

[configuration] AbstractConfiguration.addProperty doesn't split arrays

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      The addProperty method in AbstractConfiguration is responsible for adding
      multiple values separatly in the configuration, it splits Strings like "a, b,
      c", it iterates through Collections, but it doesn't handle arrays (Object[],
      int[], float[]...)

        Activity

        Hide
        Emmanuel Bourg added a comment -

        Ok, I moved fetchInsertIterator() into PropertyConverter and renamed it
        toIterator(). I added a test case to split an array of int primitives and it
        worked fine, thank you Oliver

        This allowed to remove the PropertyTokenizer class in AbstractConfiguration, it
        is now completely replaced by the split method in PropertyConverter. It's almost
        possible to have an instance specific string delimiter now

        Show
        Emmanuel Bourg added a comment - Ok, I moved fetchInsertIterator() into PropertyConverter and renamed it toIterator(). I added a test case to split an array of int primitives and it worked fine, thank you Oliver This allowed to remove the PropertyTokenizer class in AbstractConfiguration, it is now completely replaced by the split method in PropertyConverter. It's almost possible to have an instance specific string delimiter now
        Hide
        Oliver Heger added a comment -

        I was never good in finding a pregnant name So I am open for all
        suggestions. I also like the idea to remove the method to
        PropertyConverter.toIterator(). This seems to be a suitable place.

        Show
        Oliver Heger added a comment - I was never good in finding a pregnant name So I am open for all suggestions. I also like the idea to remove the method to PropertyConverter.toIterator(). This seems to be a suitable place.
        Hide
        Emmanuel Bourg added a comment -

        Ok, that's what I guessed but I wasn't sure. Could we find a better name, maybe
        getElementsIterator, iterateTokens or getTokenIterator ? Also is it better
        placed in AbstractConfiguration or in ConfigurationUtils as a static method ? It
        could also be moved to the PropertyConverter class as a toIterator(Object value)
        method.

        Show
        Emmanuel Bourg added a comment - Ok, that's what I guessed but I wasn't sure. Could we find a better name, maybe getElementsIterator, iterateTokens or getTokenIterator ? Also is it better placed in AbstractConfiguration or in ConfigurationUtils as a static method ? It could also be moved to the PropertyConverter class as a toIterator(Object value) method.
        Hide
        Oliver Heger added a comment -

        The default setProperty() implementation is not sufficient for
        HierarchicalConfiguration and HierarchicalXMLConfiguration because it first
        removes the property and then adds the values again. This changes the order of
        the elements, which was also part of the bug reports for
        XMLConfiguration.save(). Because of that I just added this new implementation of
        setProperty().

        Show
        Oliver Heger added a comment - The default setProperty() implementation is not sufficient for HierarchicalConfiguration and HierarchicalXMLConfiguration because it first removes the property and then adds the values again. This changes the order of the elements, which was also part of the bug reports for XMLConfiguration.save(). Because of that I just added this new implementation of setProperty().
        Hide
        Emmanuel Bourg added a comment -

        Could we remove the setProperty() implementation in HierarchicalConfiguration
        and use the default implementation in AbstractConfiguration ? Thus
        fetchInsertIterator could be made private.

        Show
        Emmanuel Bourg added a comment - Could we remove the setProperty() implementation in HierarchicalConfiguration and use the default implementation in AbstractConfiguration ? Thus fetchInsertIterator could be made private.
        Hide
        Oliver Heger added a comment -

        I have added this feature as a side effect of my modifications of
        HierarchicalConfiguration: I had to implement an overloaded version of the
        setProperty() method and therefor needed a way of accessing multiple property
        values to be inserted.

        I added a method fetchInsertIterator() to AbstractConfiguration, which returns
        an Iterator for all values to be inserted. It supports Strings with a split
        character, collections, arrays, single values and null values.

        Can you verify this?

        Oliver

        Show
        Oliver Heger added a comment - I have added this feature as a side effect of my modifications of HierarchicalConfiguration: I had to implement an overloaded version of the setProperty() method and therefor needed a way of accessing multiple property values to be inserted. I added a method fetchInsertIterator() to AbstractConfiguration, which returns an Iterator for all values to be inserted. It supports Strings with a split character, collections, arrays, single values and null values. Can you verify this? Oliver

          People

          • Assignee:
            Unassigned
            Reporter:
            Emmanuel Bourg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development