sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1658178 - /sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/OperationMethodSet.java
Date Sun, 08 Feb 2015 14:35:53 GMT
Author: desruisseaux
Date: Sun Feb  8 14:35:52 2015
New Revision: 1658178

URL: http://svn.apache.org/r1658178
Log:
Explain a design choice about OperationMethodSet.contains(Object).

Modified:
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/OperationMethodSet.java

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/OperationMethodSet.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/OperationMethodSet.java?rev=1658178&r1=1658177&r2=1658178&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/OperationMethodSet.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/OperationMethodSet.java
[UTF-8] Sun Feb  8 14:35:52 2015
@@ -114,6 +114,7 @@ final class OperationMethodSet extends A
                 methodIterator = null;
             }
         }
+        // Maintenance note: following check shall be consistent with the one in 'contains(Object)'.
         if (method instanceof DefaultOperationMethod) {
             if (!type.isAssignableFrom(((DefaultOperationMethod) method).getOperationType()))
{
                 return false;
@@ -206,4 +207,47 @@ final class OperationMethodSet extends A
             }
         };
     }
+
+    /**
+     * Returns {@code true} if this set contains the given object.
+     * This implementation overrides the default one with a quick check allowing us to filter
+     * {@code OperationMethod} instances of the wrong type before to iterate over the elements.
+     */
+    @Override
+    public boolean contains(final Object object) {
+        // Maintenance note: following check shall be consistent with the one in 'transfer()'.
+        if (object instanceof DefaultOperationMethod) {
+            if (!type.isAssignableFrom(((DefaultOperationMethod) object).getOperationType()))
{
+                return false;
+            }
+        } else if (!(object instanceof OperationMethod)) {
+            return false;
+        }
+        /*
+         * NOTE: we could optimize this method here with the following code, which would
be
+         * much more efficient than the default implementation if 'methods' is a HashSet:
+         *
+         *   if (methods instanceof Collection<?>) {
+         *       synchronized (methods) {
+         *           return ((Collection<?>) methods).contains(object);
+         *       }
+         *   }
+         *
+         * However we don't do that because it would bring 2 issues:
+         *
+         *   1) There is no guarantee that implementation of the 'methods' collection uses
the 'equals(Object)'
+         *      method. For example TreeSet rather uses 'compareTo(Object)'. Since the OperationMethodSet
class
+         *      uses 'equals', there is a risk of inconsistency.
+         *
+         *   2) The 'synchronized (methods)' statement introduces a risk of deadlock if some
implementations of
+         *      'OperationMethod.equals(Object)' invokes DefaultMathTransformFactory methods.
Of course no such
+         *      callbacks should exist (and Apache SIS don't do that), but the OperationMethods
can be supplied
+         *      by users and Murphy's law said that if something can go wrong, it will go
wrong.
+         *
+         * Since there is no evidence at this time that we need an efficient OperationMethodSet.contains(Object)
+         * implementation, we keep for now the slowest but more conservative approach inherited
from AbstractSet.
+         * However this choice may be revisited in any future SIS version if necessary.
+         */
+        return super.contains(object);
+    }
 }



Mime
View raw message