sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1477720 - in /sis/branches/JDK7/sis-metadata/src: main/java/org/apache/sis/metadata/ test/java/org/apache/sis/metadata/
Date Tue, 30 Apr 2013 16:44:36 GMT
Author: desruisseaux
Date: Tue Apr 30 16:44:36 2013
New Revision: 1477720

URL: http://svn.apache.org/r1477720
Log:
Simplify MetadataTreeChildren as a Collection implementation instead of List.

Modified:
    sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeChildren.java
    sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeNode.java
    sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/ValueExistencePolicy.java
    sis/branches/JDK7/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTreeChildrenTest.java

Modified: sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeChildren.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeChildren.java?rev=1477720&r1=1477719&r2=1477720&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeChildren.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeChildren.java
[UTF-8] Tue Apr 30 16:44:36 2013
@@ -16,30 +16,39 @@
  */
 package org.apache.sis.metadata;
 
-import java.util.Arrays;
 import java.util.Iterator;
-import java.util.ListIterator;
-import java.util.Collection;
 import java.util.Collections;
-import java.util.AbstractSequentialList;
+import java.util.AbstractCollection;
 import java.util.NoSuchElementException;
 import java.util.ConcurrentModificationException;
 import org.apache.sis.util.collection.TreeTable;
-import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.Debug;
 
 
 /**
- * The list of children to be returned by {@link MetadataTreeNode#getChildren()}.
- * This list holds a reference to the metadata object at creation time; it does
+ * The collection of children to be returned by {@link MetadataTreeNode#getChildren()}.
+ * This collection holds a reference to the metadata object at creation time; it does
  * not track changes in {@code parent.getUserObject()}.
  *
+ * {@section Note on value existence policy}
+ * It is better to use this class with {@link ValueExistencePolicy#NON_EMPTY} in order
+ * to avoid code complication and surprising behavior of {@link Iter#remove()} operation.
+ * If the policy is set to another value, we need to keep the following aspects in mind:
+ *
+ * <ul>
+ *   <li>When {@link Iter#hasNext()} finds a null or empty collection, it may needs
to
+ *       simulate a singleton with a null value.</li>
+ *   <li>In {@link MetadataTreeNode#getUserObject()}, we need the same check than above
+ *       for simulating a singleton collection with a null value if the node is for the
+ *       element at index 0.</li>
+ * </ul>
+ *
  * @author  Martin Desruisseaux (Geomatys)
  * @since   0.3
  * @version 0.3
  * @module
  */
-final class MetadataTreeChildren extends AbstractSequentialList<TreeTable.Node> {
+final class MetadataTreeChildren extends AbstractCollection<TreeTable.Node> {
     /**
      * The parent of the children to be returned by the iterator.
      * Some useful information are available indirectly through this parent:
@@ -53,15 +62,15 @@ final class MetadataTreeChildren extends
     private final MetadataTreeNode parent;
 
     /**
-     * The metadata object for which property values will be the elements of this list.
+     * The metadata object for which property values will be the elements of this collection.
      * This is typically an {@link AbstractMetadata} instance, but not necessarily.
      * Any type for which {@link MetadataStandard#isMetadata(Class)} returns {@code true}
is okay.
      *
-     * <p>This field is a snapshot of the {@linkplain #parent} {@link MetadataTreeNode#getUserObject()}
-     * at creation time. This list does not track changes in the reference returned by the
above-cited
+     * <p>This field is a snapshot of the {@linkplain #parent} {@link MetadataTreeNode#getUserObject()}
at
+     * creation time. This collection does not track changes in the reference returned by
the above-cited
      * {@code getUserObject()}. In other words, changes in the {@code metadata} object will
be reflected
-     * in this list, but if {@code parent.getUserObject()} returns a reference to another
object, this
-     * change will not be reflected in this list.
+     * in this collection, but if {@code parent.getUserObject()} returns a reference to another
object,
+     * this change will not be reflected in this collection.
      */
     final Object metadata;
 
@@ -77,19 +86,26 @@ final class MetadataTreeChildren extends
     private final PropertyAccessor accessor;
 
     /**
-     * The children to be returned by this list. All elements in this list are initially
-     * {@code null}, then created by {@link #childAt(int)} when first needed.
+     * The children to be returned by this collection. All elements in this collection are
+     * initially {@code null}, then created by {@link #childAt(int)} when first needed.
      *
-     * <p>Not all elements in this array will be returned by the list iterator.
+     * <p>Not all elements in this array will be returned by the iterator.
      * The value needs to be verified for the {@link ValueExistencePolicy}.</p>
      */
     private final MetadataTreeNode[] children;
 
     /**
-     * Creates a list of children for the specified metadata.
+     * Modification count, incremented when the content of this collection is modified. This
check
+     * is done on a <cite>best effort basis</cite> only, since we can't not track
the changes which
+     * are done independently in the {@linkplain #metadata} object.
+     */
+    int modCount;
+
+    /**
+     * Creates a collection of children for the specified metadata.
      *
      * @param parent   The parent for which this node is an element.
-     * @param metadata The metadata object for which property values will be the elements
of this list.
+     * @param metadata The metadata object for which property values will be the elements
of this collection.
      * @param accessor The accessor to use for accessing the property names, types and values
of the metadata object.
      */
     MetadataTreeChildren(final MetadataTreeNode parent, final Object metadata, final PropertyAccessor
accessor) {
@@ -101,17 +117,13 @@ final class MetadataTreeChildren extends
 
     /**
      * Clears the value at the given index. The given {@code index} is relative to
-     * the {@link #accessor} indexing, <strong>not</strong> to this list index.
+     * the {@link #accessor} indexing, <strong>not</strong> to this collection.
      *
      * <p>The cleared elements may or may not be considered as removed, depending on
the
      * value policy. To check if the element shall be considered as removed (for example
      * in order to update index), invoke {@code isSkipped(value)} after this method.</p>
      *
-     * {@note We do not provide setter method because this <code>List</code>
contract
-     *        requires the values to be instances of {@code TreeTable.Node}, which is
-     *        not very convenient in the case of our list view.}
-     *
-     * @param index The index in the accessor (<em>not</em> the index in this
list).
+     * @param index The index in the accessor (<em>not</em> the index in this
collection).
      */
     final void clearAt(final int index) {
         accessor.set(index, metadata, null, false);
@@ -119,9 +131,9 @@ final class MetadataTreeChildren extends
 
     /**
      * Returns the value at the given index. The given {@code index} is relative to
-     * the {@link #accessor} indexing, <strong>not</strong> to this list index.
+     * the {@link #accessor} indexing, <strong>not</strong> to this collection.
      *
-     * @param  index The index in the accessor (<em>not</em> the index in this
list).
+     * @param  index The index in the accessor (<em>not</em> the index in this
collection).
      * @return The value at the given index. May be {@code null} or a collection.
      */
     final Object valueAt(final int index) {
@@ -131,13 +143,13 @@ final class MetadataTreeChildren extends
     /**
      * Returns {@code true} if the type at the given index is a collection. The given
      * {@code index} is relative to the {@link #accessor} indexing, <strong>not</strong>
-     * to this list index.
+     * to this collection.
      *
      * {@note We do not test <code>(value instanceof Collection)</code> because
the value
      *        could be any user's implementation. Nothing prevent users from implementing
      *        the collection interface even for singleton elements if they wish.}
      *
-     * @param  index The index in the accessor (<em>not</em> the index in this
list).
+     * @param  index The index in the accessor (<em>not</em> the index in this
collection).
      * @return {@code true} if the value at the given index is a collection.
      */
     final boolean isCollection(final int index) {
@@ -157,26 +169,26 @@ final class MetadataTreeChildren extends
 
     /**
      * Returns the child at the given index, creating it if needed. The given {@code index}
-     * is relative to the {@link #accessor} indexing, <strong>not</strong> to
this list index.
+     * is relative to the {@link #accessor} indexing, <strong>not</strong> to
this collection.
      *
      * <p>This method does not check if the child at the given index should be skipped.
      * It is caller responsibility to do such verification before this method call.</p>
      *
-     * @param  index The index in the accessor (<em>not</em> the index in this
list).
-     * @param  childIndex If the property at {@link #index} is a collection, the index
-     *         in that collection (<em>not</em> the index in this list). Otherwise
-1.
-     * @return The node to be returned by pulic API.
+     * @param  index The index in the accessor (<em>not</em> the index in this
collection).
+     * @param  subIndex If the property at {@link #index} is a collection, the index in that
+     *         collection (<em>not</em> the index in <em>this</em>
collection). Otherwise -1.
+     * @return The node to be returned by public API.
      */
-    final MetadataTreeNode childAt(final int index, final int childIndex) {
+    final MetadataTreeNode childAt(final int index, final int subIndex) {
         MetadataTreeNode node = children[index];
-        if (childIndex >= 0) {
+        if (subIndex >= 0) {
             /*
              * If the value is an element of a collection, we will cache only the last used
value.
              * We don't cache all elements in order to avoid yet more complex code, and this
cover
              * the majority of cases where the collection has only one element anyway.
              */
-            if (node == null || ((MetadataTreeNode.CollectionElement) node).indexInList !=
childIndex) {
-                node = new MetadataTreeNode.CollectionElement(parent, metadata, accessor,
index, childIndex);
+            if (node == null || ((MetadataTreeNode.CollectionElement) node).indexInList !=
subIndex) {
+                node = new MetadataTreeNode.CollectionElement(parent, metadata, accessor,
index, subIndex);
             }
         } else {
             /*
@@ -195,15 +207,15 @@ final class MetadataTreeChildren extends
     /**
      * Returns the maximal number of children. This is the number of all possible elements
      * according the {@link #accessor}, including {@linkplain #isSkipped(Object) skipped}
-     * ones. This is <strong>not</strong> the list size.
+     * ones. This is <strong>not</strong> the collection size.
      */
     final int childCount() {
         return children.length;
     }
 
     /**
-     * Returns the number of elements in this list, ignoring the {@link #isSkipped(Object)
-     * skipped} ones.
+     * Returns the number of elements in this collection,
+     * ignoring the {@link #isSkipped(Object) skipped} ones.
      */
     @Override
     public int size() {
@@ -211,7 +223,7 @@ final class MetadataTreeChildren extends
     }
 
     /**
-     * Returns {@code true} if this list contains no elements. Invoking this method is more
efficient
+     * Returns {@code true} if this collection contains no elements. Invoking this method
is more efficient
      * than testing {@code size() == 0} because this method does not iterate over all properties.
      */
     @Override
@@ -220,7 +232,7 @@ final class MetadataTreeChildren extends
     }
 
     /**
-     * Returns an iterator over the nodes in the list of children.
+     * Returns an iterator over the nodes in the collection of children.
      */
     @Override
     public Iterator<TreeTable.Node> iterator() {
@@ -228,32 +240,22 @@ final class MetadataTreeChildren extends
     }
 
     /**
-     * Returns a bidirectional iterator over the nodes in the list of children.
+     * The iterator over the elements in the enclosing {@link MetadataTreeChildren} collection.
+     * Each element is identified by its index in the {@link PropertyAccessor}, together
with
+     * its position in its sub-iterator when the metadata property is a collection.
+     *
+     * {@section Implementation note}
+     * It could be cheaper to not take an iterator for the properties that are collections,
+     * and instead just increment a "sub-index" from 0 to the collection size.  It would
be
+     * cheaper because we don't really need to extract the values of those collections (i.e.
+     * the {@link #nextValue} field is not really needed). Nevertheless we prefer (for now)
+     * the iterator approach anyway because it makes easier to implement the {@link #remove()}
+     * method and has the desired side-effect to check for concurrent modifications. It also
+     * keeps the {@link #nextValue} field up-to-date in case we would like to use it in a
+     * future SIS version. We do that on the assumption that sub-iterators are cheap since
+     * they are {@code ArrayList} iterators in the majority of cases.
      */
-    @Override
-    public ListIterator<TreeTable.Node> listIterator() {
-        return new BiIter();
-    }
-
-    /**
-     * Returns an iterator over the nodes in the list of children, starting at the given
index.
-     */
-    @Override
-    public ListIterator<TreeTable.Node> listIterator(final int index) {
-        if (index >= 0) {
-            final BiIter it = new BiIter();
-            if (it.skip(index)) {
-                return it;
-            }
-        }
-        throw new IndexOutOfBoundsException(Errors.format(Errors.Keys.IndexOutOfBounds_1,
index));
-    }
-
-    /**
-     * The iterator returned by {@link #iterator()}. This iterator fetches metadata property
-     * values and creates the nodes only when first needed.
-     */
-    private class Iter implements Iterator<TreeTable.Node> {
+    private final class Iter implements Iterator<TreeTable.Node> {
         /**
          * Index in {@link MetadataTreeChildren#accessor} of the next element to be
          * returned by {@link #next()}, or {@link PropertyAccessor#count()} if we
@@ -274,9 +276,14 @@ final class MetadataTreeChildren extends
         private boolean isNextVerified;
 
         /**
-         * The value to be returned by {@link #next()} method. This value is computed ahead
-         * of time by {@link #hasNext()} since we need that information in order to determine
+         * The value of the node to be returned by the {@link #next()} method. This value
is computed
+         * ahead of time by {@link #hasNext()} since we need that information in order to
determine
          * if the value needs to be skipped or not.
+         *
+         * {@note Actually we don't really need to keep this value, since it is not used
outside the
+         *        <code>hasNext()</code> method. We keep it for now as an opportunist
information,
+         *        in case we have some need for it in a future version. For example we may
consider
+         *        to add an "original value" column in the table.}
          */
         private Object nextValue;
 
@@ -284,21 +291,20 @@ final class MetadataTreeChildren extends
          * If the call to {@link #next()} found a collection, the iterator over the elements
          * in that collection. Otherwise {@code null}.
          *
-         * <p>A non-null value (even if that child iterator has no next elements) means
that
-         * {@link #nextValue} is an element of that child iteration.</p>
+         * <p>A non-null value (even if that sub-iterator has no next elements)
+         * means that {@link #nextValue} is an element of that sub-iteration.</p>
          */
-        private Iterator<?> childIterator;
+        private Iterator<?> subIterator;
 
         /**
-         * Position of the {@link #nextValue} in the {@link #childIterator}.
-         * This field has no meaning if the child iterator is null.
+         * Position of the {@link #nextValue} in the {@link #subIterator},
+         * or -1 if the sub-iterator is null.
          */
-        private int childIndex;
+        private int subIndex = -1;
 
         /**
-         * The value of {@link AbstractSequentialList#modCount} at construction time or
-         * after the last change done by this iterator. Used for concurrent modification
-         * checks.
+         * The value of {@link MetadataTreeChildren#modCount} at construction time or after
+         * the last change done by this iterator. Used for concurrent modification checks.
          *
          * {@note Actually this iterator should be robust to most concurrent modifications.
          *        But we check anyway in order to prevent concurrent modifications in user
@@ -326,7 +332,7 @@ final class MetadataTreeChildren extends
          * Ensures that {@link #nextInAccessor} is valid. If the index has not been validated,
then this method
          * moves the iterator to the next valid element, starting at the current {@link #nextInAccessor}
value.
          *
-         * @return {@code true} on success, or {@code false} if we reached the end of the
list.
+         * @return {@code true} on success, or {@code false} if we reached the end of the
iteration.
          */
         @Override
         public boolean hasNext() {
@@ -335,23 +341,24 @@ final class MetadataTreeChildren extends
                 return true;
             }
             /*
-             * If an iteration was under progress, move to the next element from that iteration.
-             * We do not check for 'isSkipped(value)' here because empty elements in collections
-             * are probably mistakes, and we want to see them.
+             * If we were iterating over the elements of a sub-collection, move to the next
element
+             * in that iteration. We do not check for 'isSkipped(value)' here because null
or empty
+             * elements in collections are probably mistakes, and we want to see them.
              */
-            if (childIterator != null) {
-                if (childIterator.hasNext()) {
-                    childIndex++;
-                    nextValue = childIterator.next();
+            if (subIterator != null) {
+                if (subIterator.hasNext()) {
+                    nextValue = subIterator.next();
+                    subIndex++;
                     isNextVerified = true;
                     return true;
                 }
-                childIterator = null;
+                subIterator = null;
+                subIndex = -1;
                 nextInAccessor++; // See the comment before nextInAccessor++ in the next()
method.
             }
             /*
              * Search for the next property, which may be either a singleton or the first
element
-             * of a collection. In the later case, we will create a child iterator.
+             * of a sub-collection. In the later case, we will create a sub-iterator.
              */
             final int count = childCount();
             while (nextInAccessor < count) {
@@ -365,16 +372,16 @@ final class MetadataTreeChildren extends
                          * would have returned 'true'.
                          */
                         if (nextValue != null) {
-                            childIterator = ((Collection<?>) nextValue).iterator();
+                            subIterator = ((Iterable<?>) nextValue).iterator();
                         } else {
-                            childIterator = Collections.emptyIterator();
+                            subIterator = Collections.emptyIterator();
                             // Null collections are illegal (it shall be empty collections
instead),
                             // but we try to keep the iterator robut to ill-formed metadata,
because
                             // we want AbstractMetadata.toString() to work so we can spot
problems.
                         }
-                        childIndex = 0;
-                        if (childIterator.hasNext()) {
-                            nextValue = childIterator.next();
+                        subIndex = 0;
+                        if (subIterator.hasNext()) {
+                            nextValue = subIterator.next();
                         } else {
                             nextValue = null;
                             // Do not set 'childIterator' to null, since the above 'nextValue'
@@ -397,10 +404,9 @@ final class MetadataTreeChildren extends
         @Override
         public TreeTable.Node next() {
             if (hasNext()) {
-                final boolean isElementOfCollection = (childIterator != null);
-                final MetadataTreeNode node = childAt(nextInAccessor, isElementOfCollection
? childIndex : -1);
+                final MetadataTreeNode node = childAt(nextInAccessor, subIndex);
                 previousInAccessor = nextInAccessor;
-                if (!isElementOfCollection) {
+                if (subIterator == null) {
                     /*
                      * If we are iterating over the elements in a collection, the PropertyAccessor
index
                      * still the same and will be incremented by 'hasNext()' only when the
iteration is
@@ -419,7 +425,8 @@ final class MetadataTreeChildren extends
         /**
          * Clears the element returned by the last call to {@link #next()}.
          * Whether the cleared element is considered removed or not depends
-         * on the value policy and on the element type.
+         * on the value policy and on the element type. With the default
+         * {@code NON_EMPTY} policy, the effect is a removal.
          */
         @Override
         public void remove() {
@@ -427,8 +434,8 @@ final class MetadataTreeChildren extends
                 throw new IllegalStateException();
             }
             checkConcurrentModification();
-            if (childIterator != null) {
-                childIterator.remove();
+            if (subIterator != null) {
+                subIterator.remove();
             } else {
                 clearAt(previousInAccessor);
                 previousInAccessor = -1;
@@ -438,146 +445,9 @@ final class MetadataTreeChildren extends
     }
 
     /**
-     * The bidirectional iterator returned by {@link #listIterator(int)}.
-     */
-    private final class BiIter extends Iter implements ListIterator<TreeTable.Node>
{
-        /**
-         * The previous elements returned by this iterator.
-         */
-        private TreeTable.Node[] previous;
-
-        /**
-         * Number of valid elements in the {@link #previous} array.
-         * This is the position of the {@link Iter} super-class.
-         */
-        private int position;
-
-        /**
-         * Index to be returned by {@link #nextIndex()}.
-         *
-         * @see #nextIndex()
-         * @see #previousIndex()
-         */
-        private int nextIndex;
-
-        /**
-         * Creates a new iterator.
-         */
-        BiIter() {
-            previous = new TreeTable.Node[childCount()];
-        }
-
-        /**
-         * Skips the given amount of elements. This is a convenience method
-         * for implementation of {@link MetadataTreeChildren#listIterator(int)}.
-         *
-         * @param  n Number of elements to skip.
-         * @return {@code true} on success, or {@code false} if the list doesn't contain
enough elements.
-         */
-        boolean skip(int n) {
-            while (--n >= 0) {
-                if (!super.hasNext()) {
-                    return false;
-                }
-                next();
-            }
-            return true;
-        }
-
-        /**
-         * Returns the index of the element to be returned by {@link #next()},
-         * or the list size if the iterator is at the end of the list.
-         */
-        @Override
-        public int nextIndex() {
-            return nextIndex;
-        }
-
-        /**
-         * Returns the index of the element to be returned by {@link #previous()},
-         * or -1 if the iterator is at the beginning of the list.
-         */
-        @Override
-        public int previousIndex() {
-            return nextIndex - 1;
-        }
-
-        /**
-         * Returns {@code true} if {@link #next()} can return an element.
-         */
-        @Override
-        public boolean hasNext() {
-            return (nextIndex < position) || super.hasNext();
-        }
-
-        /**
-         * Returns {@code true} if {@link #previous()} can return an element.
-         */
-        @Override
-        public boolean hasPrevious() {
-            checkConcurrentModification();
-            return nextIndex != 0;
-        }
-
-        /**
-         * Returns the next element.
-         */
-        @Override
-        public TreeTable.Node next() {
-            if (nextIndex < position) {
-                checkConcurrentModification();
-                return previous[nextIndex++];
-            }
-            final TreeTable.Node node = super.next();
-            if (nextIndex == previous.length) {
-                previous = Arrays.copyOf(previous, nextIndex*2);
-            }
-            previous[nextIndex++] = node;
-            position = nextIndex;
-            return node;
-        }
-
-        /**
-         * Returns the previous element.
-         */
-        @Override
-        public TreeTable.Node previous() {
-            if (hasPrevious()) {
-                return previous[--nextIndex];
-            }
-            throw new NoSuchElementException();
-        }
-
-        /**
-         * Current implementation does not support removal after {@link #previous()}.
-         */
-        @Override
-        public void remove() {
-            if (nextIndex != position) {
-                throw new UnsupportedOperationException(Errors.format(Errors.Keys.UnsupportedOperation_1,
"remove"));
-            }
-            super.remove();
-        }
-
-        /**
-         * Unsupported operation.
-         */
-        @Override
-        public void set(TreeTable.Node e) {
-            throw new UnsupportedOperationException(Errors.format(Errors.Keys.UnsupportedOperation_1,
"set"));
-        }
-
-        /**
-         * Unsupported operation.
-         */
-        @Override
-        public void add(TreeTable.Node e) {
-            throw new UnsupportedOperationException(Errors.format(Errors.Keys.UnsupportedOperation_1,
"add"));
-        }
-    }
-
-    /**
-     * Returns a string representation of this list for debugging purpose.
+     * Returns a string representation of this collection for debugging purpose.
+     * This string representation uses one line per element instead of formatting
+     * everything on a single line.
      */
     @Debug
     @Override

Modified: sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeNode.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeNode.java?rev=1477720&r1=1477719&r2=1477720&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeNode.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/MetadataTreeNode.java
[UTF-8] Tue Apr 30 16:44:36 2013
@@ -101,11 +101,11 @@ class MetadataTreeNode implements TreeTa
     /**
      * The children of this node, or {@code null} if not yet computed. If and only if the
node
      * can not have children (i.e. {@linkplain #isLeaf() is a leaf}), then this field is
set to
-     * {@link Collections#EMPTY_LIST}.
+     * {@link Collections#EMPTY_SET}.
      *
      * @see #getChildren()
      */
-    private transient List<TreeTable.Node> children;
+    private transient Collection<TreeTable.Node> children;
 
     /**
      * Creates the root node of a new metadata tree table.
@@ -325,21 +325,37 @@ class MetadataTreeNode implements TreeTa
         }
 
         /**
-         * Fetches the node value from the metadata object.
+         * Fetches the property value from the metadata object, which is expected to be a
collection,
+         * then fetch the element at the index represented by this node.
          */
         @Override
         public Object getUserObject() {
             final Collection<?> values = (Collection<?>) super.getUserObject();
+            /*
+             * If the collection is null or empty but the value existence policy tells
+             * us that such elements shall be shown, behave as if the collection was a
+             * singleton containing a null element, in order to make the property
+             * visible in the tree.
+             */
+            if (indexInList == 0 && table.valuePolicy.substituteByNullElement(values))
{
+                return null;
+            }
             try {
                 if (values instanceof List<?>) {
                     return ((List<?>) values).get(indexInList);
                 }
                 final Iterator<?> it = values.iterator();
                 for (int i=0; i<indexInList; i++) {
-                    it.next();
+                    it.next(); // Inefficient way to move at the desired index, but hopefully
rare.
                 }
                 return it.next();
-            } catch (IndexOutOfBoundsException | NoSuchElementException e) {
+            } catch (NullPointerException | IndexOutOfBoundsException | NoSuchElementException
e) {
+                /*
+                 * May happen if the collection for this metadata property changed after
the iteration
+                 * in the MetadataTreeChildren. Users should not keep MetadataTreeNode references
+                 * instances for a long time, but instead iterate again over MetadataTreeChildren
+                 * when needed.
+                 */
                 throw new ConcurrentModificationException(e);
             }
         }
@@ -363,28 +379,28 @@ class MetadataTreeNode implements TreeTa
     }
 
     /**
-     * Returns the children of this node, or an empty list if none.
+     * Returns the children of this node, or an empty set if none.
      * Only metadata object can have children.
      */
     @Override
-    public final List<TreeTable.Node> getChildren() {
+    public final Collection<TreeTable.Node> getChildren() {
         /*
-         * 'children' is set to EMPTY_LIST if an only if the node *can not* have children,
+         * 'children' is set to EMPTY_SET if an only if the node *can not* have children,
          * in which case we do not need to check for changes in the underlying metadata.
          */
-        if (children != Collections.EMPTY_LIST) {
+        if (children != Collections.EMPTY_SET) {
             final Object value = getUserObject();
             if (value == null) {
                 /*
-                 * If there is no value, returns an empty list but *do not* set 'children'
-                 * to that list, in order to allow this method to check again the next time
+                 * If there is no value, returns an empty set but *do not* set 'children'
+                 * to that set, in order to allow this method to check again the next time
                  * that this method is invoked.
                  */
                 children = null; // Let GC do its work.
-                return Collections.emptyList();
+                return Collections.emptySet();
             }
             /*
-             * If there is a value, check if the cached list is still applicable.
+             * If there is a value, check if the cached collection is still applicable.
              */
             if (children instanceof MetadataTreeChildren) {
                 final MetadataTreeChildren candidate = (MetadataTreeChildren) children;
@@ -393,15 +409,15 @@ class MetadataTreeNode implements TreeTa
                 }
             }
             /*
-             * At this point, we need to create a new list. The property accessor will be
-             * null if the value is not a metadata object, in which case we will remember
-             * that fact by setting the children list definitively to an empty list.
+             * At this point, we need to create a new collection. The property accessor will
+             * be null if the value is not a metadata object, in which case we will remember
+             * that fact by setting the children collection definitively to an empty set.
              */
             final PropertyAccessor accessor = table.standard.getAccessor(value.getClass(),
false);
             if (accessor != null) {
                 children = new MetadataTreeChildren(this, value, accessor);
             } else {
-                children = Collections.emptyList();
+                children = Collections.emptySet();
             }
         }
         return children;

Modified: sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/ValueExistencePolicy.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/ValueExistencePolicy.java?rev=1477720&r1=1477719&r2=1477720&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/ValueExistencePolicy.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/main/java/org/apache/sis/metadata/ValueExistencePolicy.java
[UTF-8] Tue Apr 30 16:44:36 2013
@@ -41,9 +41,16 @@ public enum ValueExistencePolicy {
      * empty collection.
      */
     ALL() {
+        /** Never skip values. */
         @Override boolean isSkipped(final Object value) {
             return false;
         }
+
+        /** Substitutes null or empty collections by a null singleton element
+            in order to make the property visible in {@link MetadataTreeNode}. */
+        @Override boolean substituteByNullElement(final Collection<?> values) {
+            return (values == null) || values.isEmpty();
+        }
     },
 
     /**
@@ -51,9 +58,16 @@ public enum ValueExistencePolicy {
      * Collections are included no matter if they are empty or not.
      */
     NON_NULL() {
+        /** Skips all null values. */
         @Override boolean isSkipped(final Object value) {
             return (value == null);
         }
+
+        /** Substitutes empty collections by a null singleton element, but not
+            null references since they are supposed to be skipped by this policy. */
+        @Override boolean substituteByNullElement(final Collection<?> values) {
+            return (values != null) && values.isEmpty();
+        }
     },
 
     /**
@@ -70,9 +84,15 @@ public enum ValueExistencePolicy {
      * This is the default behavior of {@link AbstractMetadata#asMap()}.
      */
     NON_EMPTY() {
+        /** Skips all null or empty values. */
         @Override boolean isSkipped(final Object value) {
             return isNullOrEmpty(value);
         }
+
+        /** Never substitute null or empty collections since they should be skipped. */
+        @Override boolean substituteByNullElement(final Collection<?> values) {
+            return false;
+        }
     };
 
     /**
@@ -81,6 +101,16 @@ public enum ValueExistencePolicy {
     abstract boolean isSkipped(Object value);
 
     /**
+     * Returns {@code true} if {@link MetadataTreeNode} shall substitute the given collection
by
+     * a singleton containing only a null element.
+     *
+     * <p><b>Purpose:</b>
+     * When a collection is null or empty, while not excluded according this {@code ValueExistencePolicy},
+     * we need an empty space for making the metadata property visible in {@code MetadataTreeNode}.</p>
+     */
+    abstract boolean substituteByNullElement(Collection<?> values);
+
+    /**
      * Returns {@code true} if the specified object is null or an empty collection, array
or string.
      *
      * <p>This method intentionally does not inspect array or collection elements,
since this method

Modified: sis/branches/JDK7/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTreeChildrenTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTreeChildrenTest.java?rev=1477720&r1=1477719&r2=1477720&view=diff
==============================================================================
--- sis/branches/JDK7/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTreeChildrenTest.java
[UTF-8] (original)
+++ sis/branches/JDK7/sis-metadata/src/test/java/org/apache/sis/metadata/MetadataTreeChildrenTest.java
[UTF-8] Tue Apr 30 16:44:36 2013
@@ -17,10 +17,7 @@
 package org.apache.sis.metadata;
 
 import java.util.Random;
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.Iterator;
-import java.util.ListIterator;
 import org.opengis.metadata.citation.PresentationForm;
 import org.apache.sis.metadata.iso.citation.DefaultCitation;
 import org.apache.sis.util.iso.SimpleInternationalString;
@@ -98,7 +95,6 @@ public final strictfp class MetadataTree
      */
     @Test
     public void testReadOnlyWithoutCollections() {
-        random = createRandomNumberGenerator("testReadOnlyWithoutCollections");
         final DefaultCitation      citation = metadataWithoutCollections();
         final MetadataTreeChildren children = create(citation, ValueExistencePolicy.NON_EMPTY);
         final String[] expected = {
@@ -108,10 +104,7 @@ public final strictfp class MetadataTree
         };
         assertFalse ("isEmpty()", children.isEmpty());
         assertEquals("size()", expected.length, children.size());
-
         assertAllNextEqual(expected, children.iterator());
-        assertAllEqual(true, expected, children.listIterator());
-        testGet(expected, children);
     }
 
     /**
@@ -121,7 +114,6 @@ public final strictfp class MetadataTree
     @Test
     @DependsOnMethod("testReadOnlyWithoutCollections")
     public void testReadOnlyWithSingletonInCollections() {
-        random = createRandomNumberGenerator("testReadOnlyWithSingletonInCollections");
         final DefaultCitation      citation = metadataWithSingletonInCollections();
         final MetadataTreeChildren children = create(citation, ValueExistencePolicy.NON_EMPTY);
         final String[] expected = {
@@ -133,10 +125,7 @@ public final strictfp class MetadataTree
         };
         assertFalse ("isEmpty()", children.isEmpty());
         assertEquals("size()", expected.length, children.size());
-
         assertAllNextEqual(expected, children.iterator());
-        assertAllEqual(true, expected, children.listIterator());
-        testGet(expected, children);
     }
 
     /**
@@ -146,7 +135,6 @@ public final strictfp class MetadataTree
     @Test
     @DependsOnMethod("testReadOnlyWithSingletonInCollections")
     public void testReadOnlyWithMultiOccurrences() {
-        random = createRandomNumberGenerator("testReadOnlyWithMultiOccurrences");
         final DefaultCitation      citation = metadataWithMultiOccurrences();
         final MetadataTreeChildren children = create(citation, ValueExistencePolicy.NON_EMPTY);
         final String[] expected = {
@@ -160,22 +148,12 @@ public final strictfp class MetadataTree
         };
         assertFalse ("isEmpty()", children.isEmpty());
         assertEquals("size()", expected.length, children.size());
-
         assertAllNextEqual(expected, children.iterator());
-        assertAllEqual(false, expected, children.listIterator());
-        testGet(expected, children);
     }
 
 
     // ------------------------ Support methods for the above tests ------------------------
 
-
-    /**
-     * Random number generator used by the {@code assert*} methods.
-     * Must be initialized by the public test methods.
-     */
-    private Random random;
-
     /**
      * Returns the string representation of the user object in the given node.
      * This is the value that we are going compare in the assertion methods below.
@@ -198,82 +176,4 @@ public final strictfp class MetadataTree
         }
         assertFalse("Iterator.hasNext()", it.hasNext());
     }
-
-    /**
-     * Same assertion than {@link #assertAllNextEqual(String[], Iterator)},
-     * but move randomly forward and backward.
-     *
-     * @param cached {@code true} if all nodes returned by the iterator are expected to be
cached,
-     *               i.e. if asking for the element at the same index shall return the same
instance.
-     */
-    private void assertAllEqual(final boolean cached, final String[] expected, final ListIterator<TreeTable.Node>
it) {
-        final TreeTable.Node[] cache = cached ? new TreeTable.Node[expected.length] : null;
-        int index = 0; // For verification purpose only.
-        boolean forward = true;
-        for (int i=0; i<50; i++) {
-            /*
-             * Select randomly a traversal direction for this step. We reverse the
-             * direction only 1/3 of time in order to give the iterator more chances
-             * to span the full range of expected values.
-             */
-            if (index == 0) {
-                assertFalse(it.hasPrevious());
-                forward = true;
-            } else if (index == expected.length) {
-                assertFalse(it.hasNext());
-                forward = false;
-            } else if (random.nextInt(3) == 0) {
-                forward = !forward;
-            }
-            /*
-             * Get the next or previous node, depe,ding on the current traversal direction.
-             */
-            final TreeTable.Node node;
-            final String message;
-            final int at;
-            if (forward) {
-                message = "next index=" + index + " iter="+i;
-                assertEquals(message, index, it.nextIndex());
-                assertTrue(message, it.hasNext());
-                node = it.next();
-                assertEquals(message, index, it.previousIndex());
-                at = index++;
-            } else {
-                at = --index;
-                message = "previous index=" + index + " iter="+i;
-                assertEquals(message, index, it.previousIndex());
-                assertTrue(message, it.hasPrevious());
-                node = it.previous();
-                assertEquals(message, index, it.nextIndex());
-            }
-            assertEquals(message, expected[at], valueOf(node));
-            /*
-             * If the nodes are expected to be cached, verify that.
-             */
-            if (cached) {
-                if (cache[at] == null) {
-                    cache[at] = node;
-                } else {
-                    assertSame(message, cache[at], node);
-                }
-            }
-        }
-    }
-
-    /**
-     * Tests {@link MetadataTreeChildren#get(int)}, which will indirectly tests
-     * {@link MetadataTreeChildren#listIterator(int)}. The indices will be tested
-     * in random order.
-     */
-    private void testGet(final String[] expected, final MetadataTreeChildren children) {
-        final Integer[] index = new Integer[expected.length];
-        for (int i=0; i<index.length; i++) {
-            index[i] = i;
-        }
-        Collections.shuffle(Arrays.asList(index), random);
-        for (int j=0; j<index.length; j++) {
-            final int i = index[j];
-            assertEquals(expected[i], valueOf(children.get(i)));
-        }
-    }
 }



Mime
View raw message