From aadbea734d78e8d535d3a5837643336ca1384bd2 Mon Sep 17 00:00:00 2001 From: Benedikt Ritter Date: Tue, 15 Oct 2013 18:31:40 +0000 Subject: [PATCH] LANG-921 - BooleanUtils.xor(boolean...) produces wrong results git-svn-id: https://svn.apache.org/repos/asf/commons/proper/lang/trunk@1532476 13f79535-47bb-0310-9956-ffa450edef68 --- src/changes/changes.xml | 1 + .../apache/commons/lang3/BooleanUtils.java | 28 +-- .../commons/lang3/BooleanUtilsTest.java | 188 ++++++++++-------- 3 files changed, 111 insertions(+), 106 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 6665d023d..ef715e1cd 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -22,6 +22,7 @@ + BooleanUtils.xor(boolean...) produces wrong results StringUtils.normalizeSpace now handles non-breaking spaces (Unicode 00A0) Redundant check for zero in HashCodeBuilder ctor StrSubstitutor now supports default values for variables diff --git a/src/main/java/org/apache/commons/lang3/BooleanUtils.java b/src/main/java/org/apache/commons/lang3/BooleanUtils.java index 88856452e..2a7f88ff0 100644 --- a/src/main/java/org/apache/commons/lang3/BooleanUtils.java +++ b/src/main/java/org/apache/commons/lang3/BooleanUtils.java @@ -1028,14 +1028,6 @@ public static Boolean or(final Boolean... array) { * BooleanUtils.xor(true, false) = true * * - *

Note that this method behaves different from using the binary XOR operator (^). Instead of combining the given - * booleans using the XOR operator from left to right, this method counts the appearances of true in the given - * array. It will only return true if exactly one boolean in the given array is true:

- * 
-     *   true ^ true ^ false ^ true                 = true
-     *   BooleanUtils.xor(true, true, false, true)  = false
-     *
- * * @param array an array of {@code boolean}s * @return {@code true} if the xor is successful. * @throws IllegalArgumentException if {@code array} is {@code null} @@ -1050,22 +1042,13 @@ public static boolean xor(final boolean... array) { throw new IllegalArgumentException("Array is empty"); } - // Loops through array, comparing each item - int trueCount = 0; + // false if the neutral element of the xor operator + boolean result = false; for (final boolean element : array) { - // If item is true, and trueCount is < 1, increments count - // Else, xor fails - if (element) { - if (trueCount < 1) { - trueCount++; - } else { - return false; - } - } + result ^= element; } - // Returns true if there was exactly 1 true item - return trueCount == 1; + return result; } /** @@ -1077,9 +1060,6 @@ public static boolean xor(final boolean... array) { * BooleanUtils.xor(new Boolean[] { Boolean.TRUE, Boolean.FALSE }) = Boolean.TRUE * * - *

Note that this method behaves different from using the binary XOR operator (^). See - * {@link #xor(boolean...)}.

- * * @param array an array of {@code Boolean}s * @return {@code true} if the xor is successful. * @throws IllegalArgumentException if {@code array} is {@code null} diff --git a/src/test/java/org/apache/commons/lang3/BooleanUtilsTest.java b/src/test/java/org/apache/commons/lang3/BooleanUtilsTest.java index b68979424..aaacbd789 100644 --- a/src/test/java/org/apache/commons/lang3/BooleanUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/BooleanUtilsTest.java @@ -446,56 +446,68 @@ public void testXor_primitive_emptyInput() { @Test public void testXor_primitive_validInput_2items() { - assertTrue( - "True result for (true, true)", - ! BooleanUtils.xor(new boolean[] { true, true })); + assertEquals( + "true ^ true", + true ^ true , + BooleanUtils.xor(new boolean[] { true, true })); - assertTrue( - "True result for (false, false)", - ! BooleanUtils.xor(new boolean[] { false, false })); + assertEquals( + "false ^ false", + false ^ false, + BooleanUtils.xor(new boolean[] { false, false })); - assertTrue( - "False result for (true, false)", + assertEquals( + "true ^ false", + true ^ false, BooleanUtils.xor(new boolean[] { true, false })); - assertTrue( - "False result for (false, true)", + assertEquals( + "false ^ true", + false ^ true, BooleanUtils.xor(new boolean[] { false, true })); } @Test public void testXor_primitive_validInput_3items() { - assertTrue( - "False result for (false, false, true)", + assertEquals( + "false ^ false ^ false", + false ^ false ^ false, + BooleanUtils.xor(new boolean[] { false, false, false })); + + assertEquals( + "false ^ false ^ true", + false ^ false ^ true, BooleanUtils.xor(new boolean[] { false, false, true })); - assertTrue( - "False result for (false, true, false)", + assertEquals( + "false ^ true ^ false", + false ^ true ^ false, BooleanUtils.xor(new boolean[] { false, true, false })); - assertTrue( - "False result for (true, false, false)", + assertEquals( + "false ^ true ^ true", + false ^ true ^ true, + BooleanUtils.xor(new boolean[] { false, true, true })); + + assertEquals( + "true ^ false ^ false", + true ^ false ^ false, BooleanUtils.xor(new boolean[] { true, false, false })); - assertTrue( - "True result for (true, true, true)", - ! BooleanUtils.xor(new boolean[] { true, true, true })); + assertEquals( + "true ^ false ^ true", + true ^ false ^ true, + BooleanUtils.xor(new boolean[] { true, false, true })); - assertTrue( - "True result for (false, false)", - ! BooleanUtils.xor(new boolean[] { false, false, false })); + assertEquals( + "true ^ true ^ false", + true ^ true ^ false, + BooleanUtils.xor(new boolean[] { true, true, false })); - assertTrue( - "True result for (true, true, false)", - ! BooleanUtils.xor(new boolean[] { true, true, false })); - - assertTrue( - "True result for (true, false, true)", - ! BooleanUtils.xor(new boolean[] { true, false, true })); - - assertTrue( - "False result for (false, true, true)", - ! BooleanUtils.xor(new boolean[] { false, true, true })); + assertEquals( + "true ^ true ^ true", + true ^ true ^ true, + BooleanUtils.xor(new boolean[] { true, true, true })); } @Test(expected = IllegalArgumentException.class) @@ -515,35 +527,50 @@ public void testXor_object_nullElementInput() { @Test public void testXor_object_validInput_2items() { - assertTrue( - "True result for (true, true)", - ! BooleanUtils - .xor(new Boolean[] { Boolean.TRUE, Boolean.TRUE }) - .booleanValue()); - - assertTrue( - "True result for (false, false)", - ! BooleanUtils + assertEquals( + "false ^ false", + false ^ false, + BooleanUtils .xor(new Boolean[] { Boolean.FALSE, Boolean.FALSE }) .booleanValue()); - assertTrue( - "False result for (true, false)", + assertEquals( + "false ^ true", + false ^ true, + BooleanUtils + .xor(new Boolean[] { Boolean.FALSE, Boolean.TRUE }) + .booleanValue()); + + assertEquals( + "true ^ false", + true ^ false, BooleanUtils .xor(new Boolean[] { Boolean.TRUE, Boolean.FALSE }) .booleanValue()); - assertTrue( - "False result for (false, true)", + assertEquals( + "true ^ true", + true ^ true, BooleanUtils - .xor(new Boolean[] { Boolean.FALSE, Boolean.TRUE }) + .xor(new Boolean[] { Boolean.TRUE, Boolean.TRUE }) .booleanValue()); } @Test public void testXor_object_validInput_3items() { - assertTrue( - "False result for (false, false, true)", + assertEquals( + "false ^ false ^ false", + false ^ false ^ false, + BooleanUtils.xor( + new Boolean[] { + Boolean.FALSE, + Boolean.FALSE, + Boolean.FALSE }) + .booleanValue()); + + assertEquals( + "false ^ false ^ true", + false ^ false ^ true, BooleanUtils .xor( new Boolean[] { @@ -552,8 +579,9 @@ public void testXor_object_validInput_3items() { Boolean.TRUE }) .booleanValue()); - assertTrue( - "False result for (false, true, false)", + assertEquals( + "false ^ true ^ false", + false ^ true ^ false, BooleanUtils .xor( new Boolean[] { @@ -562,8 +590,9 @@ public void testXor_object_validInput_3items() { Boolean.FALSE }) .booleanValue()); - assertTrue( - "False result for (true, false, false)", + assertEquals( + "true ^ false ^ false", + true ^ false ^ false, BooleanUtils .xor( new Boolean[] { @@ -572,47 +601,42 @@ public void testXor_object_validInput_3items() { Boolean.FALSE }) .booleanValue()); - assertTrue( - "True result for (true, true, true)", - ! BooleanUtils - .xor(new Boolean[] { Boolean.TRUE, Boolean.TRUE, Boolean.TRUE }) - .booleanValue()); + assertEquals( + "true ^ false ^ true", + true ^ false ^ true, + BooleanUtils.xor( + new Boolean[] { + Boolean.TRUE, + Boolean.FALSE, + Boolean.TRUE }) + .booleanValue()); - assertTrue( - "True result for (false, false)", - ! BooleanUtils.xor( - new Boolean[] { - Boolean.FALSE, - Boolean.FALSE, - Boolean.FALSE }) - .booleanValue()); - - assertTrue( - "True result for (true, true, false)", - ! BooleanUtils.xor( + assertEquals( + "true ^ true ^ false", + true ^ true ^ false, + BooleanUtils.xor( new Boolean[] { Boolean.TRUE, Boolean.TRUE, Boolean.FALSE }) .booleanValue()); - assertTrue( - "True result for (true, false, true)", - ! BooleanUtils.xor( + assertEquals( + "false ^ true ^ true", + false ^ true ^ true, + BooleanUtils.xor( new Boolean[] { - Boolean.TRUE, Boolean.FALSE, + Boolean.TRUE, Boolean.TRUE }) .booleanValue()); - assertTrue( - "False result for (false, true, true)", - ! BooleanUtils.xor( - new Boolean[] { - Boolean.FALSE, - Boolean.TRUE, - Boolean.TRUE }) - .booleanValue()); + assertEquals( + "true ^ true ^ true", + true ^ true ^ true, + BooleanUtils + .xor(new Boolean[] { Boolean.TRUE, Boolean.TRUE, Boolean.TRUE }) + .booleanValue()); } // testAnd