ericman314 / UnitMath

JavaScript library for unit conversion and arithmetic
Apache License 2.0
29 stars 7 forks source link

valueOf should be useful for sorting #23

Open danni opened 4 years ago

danni commented 4 years ago

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.

ericman314 commented 4 years ago

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.

danni commented 4 years ago

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 notifications@github.com, 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.

josdejong commented 4 years ago

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

ericman314 commented 4 years ago

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))
harrysarson commented 4 years ago

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:

danni commented 4 years ago

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.