Skip to content

Conversation

mbostock
Copy link
Member

Fixes #107.

@Fil
Copy link
Member

Fil commented May 19, 2019

Can we make least accept an accessor as well as comparator ?

least(data, d => d.temperature)
is quicker to grasp (and type) than
least(data, (a,b) => d3.ascending(a.temperature, b.temperature))

d3.bisector accepts both https://github.com/d3/d3-array#bisector, using ascendingComparator — not sure if this is generalizable.

@mbostock
Copy link
Member Author

In that case d3.least is equivalent to d3.min. But I did have exactly that thought: instead of adding d3.least and d3.leastIndex, generalize d3.min to accept a comparator as well as an accessor, using function.length to disambiguate, and add d3.minIndex.

@mbostock
Copy link
Member Author

Ah, right. We can’t generalize d3.min because it already passes the accessor three arguments: the value, the index and the array. So, we could generalize d3.least and d3.leastIndex, but it feels unnecessary if we also have the accessor-based d3.min and d3.minIndex.

@mbostock
Copy link
Member Author

mbostock commented May 20, 2019

Okay, I’ve:

  • Added d3.maxIndex.
  • Added d3.minIndex.
  • Changed d3.leastIndex to return -1 instead of undefined.
  • Preserved d3.scan returning undefined for backwards-compatibility.
  • Added tests.
  • Updated the README.

I still need to update the README.

@mbostock mbostock marked this pull request as ready for review May 20, 2019 04:24
@Fil
Copy link
Member

Fil commented May 20, 2019

My suggestion is to add this notation:
d3.least([{ foo: 42 }, { foo: 29 }], a => a.foo)

This is imo the most concise way of writing "give me an object that has the lowest foo". (An optional comparator could then be given as a second argument.)

the code would be something like:

export default function least(values, f = ascending, g = ascending) {
  const valueof = f.length === 1 ? f : null,
    compare = f.length === 2 ? f : g;
  let min, v;
  let defined = false;
  for (const value of values) {
    v = valueof ? valueof(value) : value;
    if (defined
        ? compare(v, min) < 0
        : compare(v, v) === 0) {
      min = value;
      defined = true;
    }
  }
  return min;
}

(and leastIndex should follow the same API)

@mbostock
Copy link
Member Author

Ah, okay, that makes sense. Because unlike

d3.min([{foo: 42}, {foo: 29}], a => a.foo)

d3.least will return the element, not the value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Rename d3.scan to d3.leastIndex; add d3.least.

2 participants