Tracking issue for slice::sort_by_key

[alexcrichton]

As-is on the tin, mirrors the recently-stabilized min_by_key and max_by_key methods on Iterator.

[alexcrichton]

🔔 This issue is now entering its final comment period for stabilization 🔔

[malbarbo]

There is a function related to sort_by_key that is missing: binary_search_by_key.

sort -> binary_search
sort_by -> binary_search_by
sort_by_key -> binary_search_by_key

[alexcrichton]

An excellent point! I'll see if I can't add that soon to sneak it in for this FCP as well

[alexcrichton]

Hm actually, now that I think about it I'm not quite sure what the signature for binary_search_by_key would look like. I'm not sure it jives well with the other patterns (e.g. it's "just a change in closure arguments"), as I think it needs to look something like:

fn binary_search_by_key<B, F>(&self, b: &B, f: F) -> Result<usize, usize>
    where F: FnOnce(&Self::Item) -> B, B: Ord;

Is that what you were thinking as well?

[malbarbo]

Yes.

I suggest renaming B to K (also in sort_by_key) and b to key.

[alexcrichton]

Hm this may actually be a case where I'm not sure the extra method may buy much, for example compare:

slice.binary_search_by(|k| k.key.cmp(&val));

// vs

slice.binary_search_by_key(&val, |k| &k.key);

The Ordering-based one is not only shorter by one character but it also looks a little more readable to me. Along those lines I'd actually be hesitant to add it just yet

[malbarbo]

For search with closures that maybe the case, but for search with methods or functions binary_search_by_key is more readable

s.sort_by_key(Point::x);
// this is more readable
s.binary_search_by_key(&1, Point::x);
// than this
s.binary_search_by(|p| p.x().cmp(&1));

[alexcrichton]

Oh hm yeah, good point!

[alexcrichton]

The libs team discussed this recently and the decision was to stabilize

[alexcrichton]

Closed by #30943

[malbarbo]

Should we open another issue for binary_search_by_key or you think it's no relevant anymore?