UnitMath icon indicating copy to clipboard operation
UnitMath copied to clipboard

valueOf should be useful for sorting

Open danni opened this issue 5 years ago • 6 comments

Lodash forces objects into values via valueOf when sorting. This would be useful for sorting arrays of units, without having to pass custom compare function, etc.

danni avatar Nov 23 '19 04:11 danni

I'm definitely open to implementing valueOf, but it's not clear to me what primitive value to return. Should it be the "number" part of the unit? Or the number converted to SI units? What about units with different quantities, like inches and hours? The only way to preserve all the information about the unit is to have valueOf return more or less the same thing as toString, but then you wouldn't be able to sort them numerically.

ericman314 avatar Nov 23 '19 19:11 ericman314

Yeah I thought about this while writing a wrapper for my lodash call.

It needs to be a Number for sorting. I don’t think there’s a pure solution for mixed units but similarly the behaviour is undefined for sorting mixed types and the onus can be on the caller.

Some kind of utility function to assert the array is one type could help avoid this. In my case I enforce the type on output. On 24 Nov 2019, 06:19 +1100, Eric Mansfield [email protected], wrote:

I'm definitely open to implementing valueOf, but it's not clear to me what primitive value to return. Should it be the "number" part of the unit? Or the number converted to SI units? What about units with different quantities, like inches and hours? The only way to preserve all the information about the unit is to have valueOf return more or less the same thing as toString, but then you wouldn't be able to sort them numerically. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

danni avatar Nov 23 '19 21:11 danni

Would it make sense to use a a compare function instead? When sorting a JavaScript array with sort, you can pass a compare function.

josdejong avatar Nov 24 '19 08:11 josdejong

Yep, UnitMath already has a compare function. I don't know how it would look using lodash, but in regular JavaScript you can use:

units.sort((a, b) => a.compare(b))

ericman314 avatar Nov 24 '19 09:11 ericman314

I do not think it would make sense to sort a list of units. For instance how should [ 1m, 1kg ] be sorted?

My two cents say that .valueOf() is better left unimplemented and users can supply their own comparison function to fit their use case :+1:

harrysarson avatar Nov 24 '19 22:11 harrysarson

Yep using compare is an obvious solution if you're just sorting on one column, but doing a multi-column sort then requires writing quite a bit of code. I believe custom compare functions are being added in lodash 5, although you'd still need to set certain keys to sort on functions.

Truthfully though, sorting different units is as undefined as sorting different types, and when calling functions like orderBy you already need to be aware of this, so I think people will accept whatever strangeness they get.

To @harrysarson's point, valueOf is already implemented, it returns a string, which I think either way is surprising, as it currently allows Unit(1, 'm') + 'badger' to be valid syntax.

danni avatar Nov 24 '19 22:11 danni