checkValues() method of *MapStringValueCollections doesn't work

• Key: JDO-218
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Michelle Caisse
• Reporter: Michelle Caisse
• Component/s: tck2
• Fix Version/s: JDO 2 beta

Description

No map fields with BigDecimal values compare okay. There are several problems.

Comments

[Michelle Caisse]

The attached patch fixes the BigDecimal comparison problem. All *MapStringValueCollections tests pass using application identity with this patch applied.
The relevant new code (taking TestMapStringKeyCollections.java as an example) is:

            else if (! expected.equals(actual)) {
                if (TestUtil.getFieldSpecsForMap(
                            MapStringValueCollections.fieldSpecs[i]
                            ).get(0).equals("BigDecimal")) {
                    Set expectedKeySet = expected.keySet();
                    Set actualKeySet = actual.keySet();
                    Iterator expectedIter = expectedKeySet.iterator();
                    while (expectedIter.hasNext()) {
                        BigDecimal expectedKey = (BigDecimal) expectedIter.next();
                        // compare keys
                        if (!TestUtil.containsBigDecimalKey(expectedKey,
                                    actualKeySet)) {
                            sbuf.append("\nFor element " + i +
                                    " expected key = " + expectedKey +
                                    " not found in actual Map. Actual keyset is "
                                    + actualKeySet.toString());
                        // compare values
                        } else {
                            String expectedVal = (String) expected.get(expectedKey);
                            String actualValue = (String)
                               actual.get(TestUtil.getBigDecimalKey(expectedKey,
                                                                    actualKeySet));
                            if (!expectedVal.equals(actualValue)) {
                                sbuf.append("\nFor element " + i +
                                    " expected value = " + expectedVal +
                                    " actual Value = " + actualValue);
                           }
                       }
                    }
            }
Two new static functions were added to TestUtil.java. boolean containsBigDecimalKey(BigDecimal expectedKey, Set actualKeySet) and BigDecimal getBigDecimalKey(BigDecimal expectedKey, Set actualKeySet) use BigDecimal.compareTo to determine equality of two keys.

[Craig Russell]

It seems like this code has a similar problem in case there is a BigDecimal as a value.

+ String expectedVal = (String) expected.get(expectedKey);
+ String actualValue = (String)
+ actual.get(TestUtil.getBigDecimalKey(expectedKey,
+ actualKeySet));
+ if (!expectedVal.equals(actualValue)) {
+ sbuf.append("\nFor element " + i +
+ " expected value = " + expectedVal +
+ " actual Value = " + actualValue);
+ }

Could it be rewritten to use compareTo instead:

+ String expectedVal = (String) expected.get(expectedKey);
+ String actualValue = (String)
+ actual.get(TestUtil.getBigDecimalKey(expectedKey,
+ actualKeySet));
+ if (expectedVal.equals(actualValue)) {continue;}
if (expectedVal instanceof Comparable && ((Comparable)expectedVal.compareTo(actualValue)) == 0) {continue;}
+ sbuf.append("\nFor element " + i +
+ " expected value = " + expectedVal +
+ " actual Value = " + actualValue);
+ }

[Michelle Caisse]

The code expects only BigDecimal keys. It is for the tests where the values are all String and the keys can be any of a number of types, specifically TestMapStringValueCollections, TestHashMapStringValueCollections, TestHashtableStringValueCollections, and TestTreeMapStringValueCollections. The checkValues() method in the Test*MapStringKeyCollections is different, but in those tests there would be only String keys.

It would be great to be able to generalize this method and pull the code out of the fieldtypes tests, but it is tricky because the data type of the Collection appears in the test. There are also casting and polymorphism issues which vary among the Collection (not Map) tests when iterating through the BigDecimal fields and make this difficult.

[Craig Russell]

I totally missed that the expectedVal and actualVal were cast as Strings! My comments would apply to a more general method but not to this case.

-20-

[Michelle Caisse]

revision 345613

[Michael Bouschen]

Reopened to set the Fix Version/s field to JDO 2 beta.