Uploaded image for project: 'Groovy'
  1. Groovy
  2. GROOVY-1553

I fixed some problems with ranges and added some additional unit tests.

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • None
    • 1.1-beta-2
    • groovy-jdk
    • None
    • Max OS X 10.4
    • Patch

    Description

      I added some additional unit tests and JavaDoc for ranges and fixed some problems that I found.

      I added step() to the Range interface. It was present in both ObjectRange and IntRange but not in Range. Guillaume approved this change via email.

      Hash codes were being computed using only "reverse," "from," and "to." This results in hash codes that are inconsistent with the contract specified by List.hashCode(). As a result, range.equals(list) was true but range.hashCode() == list.hashCode() was false. I removed IntRange.hashCode() and ObjectRange.hashCode() so the use AbstractList.hashCode() instead. This fixes this problem although it is less efficient, especially for large ranges.

      I added a checks for null in equals(). equals(null) should return false rather than throwing a NullPointerException.

      I made subList(i, i) return an empty list instead of a single element list. subList() is from a to b, exclusive, so it should return an empty list in this case.

      I made contains(Object) consistent. IntRange had been returning true if the argument was an element of the range or was a sub-range. ObjectRange had a contains(Comparable) but no contains(Object). ObjectRange.contains(Comparable) returned true if the argument was between from and to and didn't consider sub ranges. The expected behavior, according to the List interface is to return true if the argument is an element of the list. RangeTest tested for this behavior in ObjectRange, making sure a range of BigDecimals from 1.1 to 9.1 didn't contain 8.0. But the test only passed because ObjectRange.contains(Comparable) wasn't called because AbstractList.contains(Object) was called instead.

      Anyway, since the List contract says contains(Object) should return true if and only if the argument is a member of the list, I made IntRange.contains() do this and removed ObjectRange.contains(Comparable). I also added IntRange.containsAll() which overrides AbstractList.containsAll() to look for sub ranges efficiently.

      I made ranges of Shorts and Floats promote their 'from' value to Integer and Double, respectively. Previously, the from value had been of the original type and all the remaining values the promoted type, as the remaining values were created by incrementing the from value. I think the original version had ClassCastException problems when it tried to compare from with other values in the range. Also, having all values in the range the same type seemed to be more consistent and make more sense.

      I removed some uses of InvokerHelper.compareEqual() to be consistent with list. I think compareEqual() is both more and less forgiving than equals(). I think it says Long(0) is equal to Integer(0), for example. Other lists, however, use equals() and equals() is supposed to be symmetric. So, using compareEqual() you get situations where range.equals(list) but !list.equals(range). Also, compareEqual() is sometimes less forgiving than equals(). I think asking a range of integers whether it contains the string "hello", for example, threw a NumberFormatException instead of returning false.

      RangeTestSuite runs all the range-related tests.

      This issue replaces GROOVY-1546. Please disregard the patches in that issue.

      Attachments

        1. empty_range.patch
          20 kB
          Edwin Tellman
        2. range.patch
          77 kB
          Edwin Tellman

        Activity

          People

            paulk Paul King
            etellman Edwin Tellman
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: