Groovy way of finding largest matching list member based on members property

Is there a better way to find closest object with lesser marker?

class Prop implements Comparable {
  BigDecimal marker
  String value

  int compareTo(oth开发者_如何学运维er) {
    return marker <=> other.marker
  }

}

def props = [new Prop(marker: 0G, value: "1st"),
new Prop(marker: 20G, value: "2nd"),
new Prop(marker: 30G, value: "3rd"),
new Prop(marker: 40G, value: "4th"),
new Prop(marker: 50G, value: "5th"),
new Prop(marker: 100G, value: "6th"),
new Prop(marker: 1000G, value: "7th")]

def whereAmI = 44G

def here = props.findAll{it.marker <= whereAmI}.max()

---

Edit: Updated to return the correct object type, and nulls.

Assuming that order isn't guaranteed, you could use the inject method:

// Inject null, so if nothing is lower than max, this returns null
def here = props.inject(null){ max, it ->
    (it.marker <= whereAmI && (!max || it > max)) ? it : max
}

If the list is always sorted, then simply use this:

// Inject null, so if nothing is lower than max, this returns null
def here = props.inject(null){ max, it ->
    (it.marker <= whereAmI) ? it : max
}

The benefit here is you only loop over the set once, and you don't create an additional interim List of lesser values.

Now, depending on your list size, the argument may be that your original example is a lot easier to read, and a lot clearer. Clarity in code can trump performance.

(Note: inject is Groovy's version of reduce or fold.)

---

Here's an alternative to OverZealous's inject solution that works if the props list is sorted by marker. Create a takeWhile method on Lists, and then take the last one less than whereAmI:

List.metaClass.takeWhile = { closure ->
    def iter = delegate.iterator()
    int i = 0
    def n = iter.next()
    while(closure.call(n)) {
        i++
        if (iter.hasNext())
            n = iter.next()
        else
            break
    }
    return delegate[0..<i]
}   

props.takeWhile { it.marker < whereAmI }[-1]           // exception if nothing matches
props.takeWhile { it.marker < whereAmI }.reverse()[0]  // null if nothing matches

---

What a better way to do it is will depend on what you consider as better.

For code readability, I think the solution you originally proposed is very good.

The OverZealous solution migth be faster but, as he mentioned, it's not as readable. And in fact, if performance really matters for that code, you should profile it to see if it's really faster.

If the props list is created once (or few times) but the here value is calculated many times, you migth consider sorting props and looking for whereAmI with a binary search. This will take log(n) time (n the size of props) instead of linear time.

// Make whereAmI a Prop to avoid defining a Comparator
def whereAmI = new Prop(marker: 44G, value: '')
def i = Collections.binarySearch(props, whereAmI)
def here = props[i >= 0 ? i : -i - 2]

When whereAmI is not in props, bynarySearch returns the negative of one plus the index where whereAmI should be, hence the seemingly magical -1 -2 there.

Warning: this won't work if there is no element in props that is less than whereAmI. if that is a possibile case, you should ask for a i == -1. The original code assigned null to here in that case:

def here = i == -1 ? null : props[i >= 0 ? i : -i - 2]