Commons Collections
  1. Commons Collections
  2. COLLECTIONS-322

Adds a Collections wrapper around the w3c NodeList

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-alpha1, 4.0
    • Component/s: List
    • Labels:
      None

      Description

      org.w3c.dom.NodeList is defined as an "abstract collection of Nodes" and java.util.List is defined as "An ordered collection (also known as a sequence). The user of this interface has precise control over where in the list each element is inserted. The user can access elements by their integer index (position in the list), and search for elements in the list.". It seemed similar enough, so I did an implementation of the useful methods, while throwing the appropriate exception when the method wouldn't make sense.

      1. TestNodeListAsCollection.java
        4 kB
        Hasan Diwan
      2. patch.txt
        10 kB
        Thomas Vahrst
      3. nodelistAsIterable.patch
        14 kB
        Thomas Vahrst
      4. NodeListAsCollection.java
        3 kB
        Hasan Diwan

        Activity

        Hide
        Hasan Diwan added a comment -

        File does need the apache header on it, but after that, it's clear to go.

        Show
        Hasan Diwan added a comment - File does need the apache header on it, but after that, it's clear to go.
        Hide
        Henri Yandell added a comment -

        Needs unit test, but otherwise seems like a nice idea.

        Show
        Henri Yandell added a comment - Needs unit test, but otherwise seems like a nice idea.
        Hide
        Hasan Diwan added a comment -

        Updated class along with unit tests.

        Show
        Hasan Diwan added a comment - Updated class along with unit tests.
        Hide
        Hasan Diwan added a comment -

        Refactor unit test

        Show
        Hasan Diwan added a comment - Refactor unit test
        Hide
        Thomas Vahrst added a comment - - edited

        proposal for an implementation for NodeListAsList, with Junit Tests.

        NodeListAsList implements the Unmodifiable interface, because the org.w3c.NodeList has not support for adding/removing items.
        In most cases the org.w3c.NodeList is used to iterate over the children of a parent node. So it may be a better idea to provide a NodeListUtils class with methods like

          Iterable<Node> NodeListUtils.asIterable(org.w3c.NodeList)  
          Iterable<Node> NodeListUtils.getChildNodesAsIterable(org.w3c.NodeList)
        

        instead of providing a 'crippled' List implementation (NodeListAsList). NodeListAsList could then be made a private inner class of NodeListUtils.

        Show
        Thomas Vahrst added a comment - - edited proposal for an implementation for NodeListAsList, with Junit Tests. NodeListAsList implements the Unmodifiable interface, because the org.w3c.NodeList has not support for adding/removing items. In most cases the org.w3c.NodeList is used to iterate over the children of a parent node. So it may be a better idea to provide a NodeListUtils class with methods like Iterable<Node> NodeListUtils.asIterable(org.w3c.NodeList) Iterable<Node> NodeListUtils.getChildNodesAsIterable(org.w3c.NodeList) instead of providing a 'crippled' List implementation (NodeListAsList). NodeListAsList could then be made a private inner class of NodeListUtils.
        Hide
        Thomas Neidhart added a comment -

        Hi Thomas,

        I looked at your patch and it looks pretty good, thanks for that.
        As you pointed out yourself, maybe a better solution would be to provide static helper methods to more easily iterate on a w3c DOM list.

        Feel free to provide a patch for that too.

        Thanks,

        Thomas

        Show
        Thomas Neidhart added a comment - Hi Thomas, I looked at your patch and it looks pretty good, thanks for that. As you pointed out yourself, maybe a better solution would be to provide static helper methods to more easily iterate on a w3c DOM list. Feel free to provide a patch for that too. Thanks, Thomas
        Hide
        Thomas Vahrst added a comment - - edited

        I implemented the suggested Utility methods, which provide an Iterable for a given NodeList or ParentNode. See nodelistAsIterable.patch

        I chose IteratorUtils as implementation class, I found this class matching best for the new services.

        The util methods now allow easy iteration over org.w3c.NodeLists or ChildNodes of a given parent node:

        Node parentNode = ...;
        
        for(Node childNode : IteratorUtils.asIterable(parentNode){
          ... do something;
        }
        
        
        Show
        Thomas Vahrst added a comment - - edited I implemented the suggested Utility methods, which provide an Iterable for a given NodeList or ParentNode. See nodelistAsIterable.patch I chose IteratorUtils as implementation class, I found this class matching best for the new services. The util methods now allow easy iteration over org.w3c.NodeLists or ChildNodes of a given parent node: Node parentNode = ...; for (Node childNode : IteratorUtils.asIterable(parentNode){ ... do something; }
        Hide
        Thomas Neidhart added a comment -

        Applied the patch with some modifications in r1438955:

        • javadoc and formatting
        • in IteratorUtils, I changed the method name to nodeListIterator and it returns an Iterator, this can be used as input for asIterable.

        Thanks for your patch!

        Show
        Thomas Neidhart added a comment - Applied the patch with some modifications in r1438955: javadoc and formatting in IteratorUtils, I changed the method name to nodeListIterator and it returns an Iterator, this can be used as input for asIterable. Thanks for your patch!
        Hide
        Thomas Vahrst added a comment -

        There is a small sample code in the javadoc of IteratorUtils.nodelistIterator(Node) which doesn't match.:

            * Convenience method, allows easy iteration over NodeLists:
             * <pre>
             *   for(Node childNode : IteratorUtils.asIterable(node)){
             *     ...
             *   }
             * </pre>
        
        

        Should now be :

            * Convenience method, allows easy iteration over NodeLists:
             * <pre>
             *   Iterable<Node> iterable = IteratorUtils.nodeListIterator(node);
             *   for(Node childNode : IteratorUtils.asIterable(iterable)){
             *     ...
             *   }
             * </pre>
        
        

        ... or perhaps better using the iterator in a while loop:

            * Convenience method, allows easy iteration over NodeLists:
             * <pre>
             *  Iterator<Node> iterator = IteratorUtils.nodeListIterator(nodeList);
             *  while(iterator.hasNext()){
             *      Node childNode = iterator.next();
             *      ...
             *  }
             * </pre>
        
        Show
        Thomas Vahrst added a comment - There is a small sample code in the javadoc of IteratorUtils.nodelistIterator(Node) which doesn't match.: * Convenience method, allows easy iteration over NodeLists: * <pre> * for (Node childNode : IteratorUtils.asIterable(node)){ * ... * } * </pre> Should now be : * Convenience method, allows easy iteration over NodeLists: * <pre> * Iterable<Node> iterable = IteratorUtils.nodeListIterator(node); * for (Node childNode : IteratorUtils.asIterable(iterable)){ * ... * } * </pre> ... or perhaps better using the iterator in a while loop: * Convenience method, allows easy iteration over NodeLists: * <pre> * Iterator<Node> iterator = IteratorUtils.nodeListIterator(nodeList); * while (iterator.hasNext()){ * Node childNode = iterator.next(); * ... * } * </pre>
        Hide
        Thomas Neidhart added a comment -

        Ok thanks, javadoc has been corrected, and I also added since tags.

        We may also add explicit IteratorUtils.asIterable methods for convenience as you proposed.
        I wanted to keep the generic way:

        • add static factory method to create a specific iterator (over NodeList)
        • use generic asIterable(Iterator) to create the actual Iterable object

        Of course, this means you have to type more, but would this be acceptable?

        Show
        Thomas Neidhart added a comment - Ok thanks, javadoc has been corrected, and I also added since tags. We may also add explicit IteratorUtils.asIterable methods for convenience as you proposed. I wanted to keep the generic way: add static factory method to create a specific iterator (over NodeList) use generic asIterable(Iterator) to create the actual Iterable object Of course, this means you have to type more, but would this be acceptable?
        Hide
        Thomas Vahrst added a comment -

        I think, the generic way you propose is ok. I suggest to close the issue.
        Thomas.

        Show
        Thomas Vahrst added a comment - I think, the generic way you propose is ok. I suggest to close the issue. Thomas.

          People

          • Assignee:
            Unassigned
            Reporter:
            Hasan Diwan
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development