Stop misuse of the API and ensure only one flag is set at once. This …

…is less powerful but stops users setting custom flags (e.g. 999 in Asad's ReactionDecoder) which has undefined behaviour.
johnmay · Nov 9, 2016 · b5ff6c4 · b5ff6c4
1 parent 7897397
commit b5ff6c4
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 2 deletions.
diff --git a/base/data/src/main/java/org/openscience/cdk/ChemObject.java b/base/data/src/main/java/org/openscience/cdk/ChemObject.java
@@ -359,11 +359,17 @@ public void setID(String identifier) {
         notifyChanged();
     }
 
+    private boolean isPowerOfTwo(int num) {
+        return (num == 1) || (num & (num-1)) == 0;
+    }
+
     /**
-     * @inheritDoc
+     * {@inheritDoc}
      */
     @Override
     public void setFlag(int mask, boolean value) {
+        if (mask > Short.MAX_VALUE || !isPowerOfTwo(mask))
+            throw new IllegalArgumentException("setFlag() must be provided a valid CDKConstant and not used for custom properties");
         // set/unset a bit in the flags value
         if (value)
             flags |= mask;

diff --git a/base/interfaces/src/main/java/org/openscience/cdk/interfaces/IChemObject.java b/base/interfaces/src/main/java/org/openscience/cdk/interfaces/IChemObject.java
@@ -205,7 +205,9 @@ public interface IChemObject extends ICDKObject {
     /**
      * Sets the value of some flag. The flag is a mask from a given
      * CDKConstant (e.g. {@link org.openscience.cdk.CDKConstants#ISAROMATIC}
-     * or {@link org.openscience.cdk.CDKConstants#VISITED}).
+     * or {@link org.openscience.cdk.CDKConstants#VISITED}). The flags are
+     * intrinsic internal properties and should not be used to store custom
+     * values, please use {@link #setProperty(Object, Object)}.
      *
      * <pre>{@code
      * // set this chem object to be aromatic

diff --git a/base/silent/src/main/java/org/openscience/cdk/silent/ChemObject.java b/base/silent/src/main/java/org/openscience/cdk/silent/ChemObject.java
@@ -302,11 +302,17 @@ public void setID(String identifier) {
         this.identifier = identifier;
     }
 
+    private boolean isPowerOfTwo(int num) {
+        return (num == 1) || (num & (num-1)) == 0;
+    }
+
     /**
      * @inheritDoc
      */
     @Override
     public void setFlag(int mask, boolean value) {
+        if (mask > Short.MAX_VALUE || !isPowerOfTwo(mask))
+            throw new IllegalArgumentException("setFlag() must be provided a valid CDKConstant and not used for custom properties");
         // set/unset a bit in the flags value
         if (value)
             flags |= mask;