Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Three arguments of the same type is confusing #4

Open
hax opened this issue Jan 17, 2024 · 21 comments
Open

Three arguments of the same type is confusing #4

hax opened this issue Jan 17, 2024 · 21 comments

Comments

@hax
Copy link
Member

hax commented Jan 17, 2024

The function currently includes three arguments of the same type, making it difficult to determine the specific meanings of each parameter when calling the function as Math.clamp(a, b, c). Both the parameter order (value, min, max) and (min, value, max) can confuse some developers because they may expect the opposite.

It might be worth considering alternative approaches such as Math.clamp(value, { min, max }) or value.clamp(min, max). These alternatives provide a clearer way of expressing the function parameters.

@Richienb Richienb changed the title three arguments with same type is confusing Three arguments of the same type is confusing Jan 17, 2024
@Richienb
Copy link
Collaborator

Richienb commented Jan 17, 2024

value.clamp might be out of the question because there only exist to* methods on Number.prototype.

As for {min, max}, objects are already used in some navigator functions so it wouldn't be that unjavascripty. It would also allow either min or max to be left unspecified so as to allow for more explicit forms: Math.clamp(value, {min: 0}) === Math.max(0, value).

@Richienb
Copy link
Collaborator

Following #5 (comment), we can name the arguments number, min and max.

@Richienb
Copy link
Collaborator

What about if neither min nor max is provided by the user?
What about if no object at all is provided by the user?

Should Math.clamp return the number untouched, or throw a TypeError?

@theScottyJam
Copy link

It is true that all parameters are numeric types, but generally you'd still be able to easily figure out what the order is by reading the code.

For example:

character.pos.x = Math.clamp(character.pos.x, 0, SCREEN_WIDTH);

Or:

return Math.clamp(angle + offset, 0, 360);

I don't think it's too difficult in those examples to figure out which parameter does what, even if you would expect Math.clamp()'s function signature to be a little different.

That being said, I'm not overly tied to this stance - I could see some potential readability benefit to explicitly labeling "min" and "max", even if it's pretty easy to figure out which one does which.

(I also don't find the use-case of omitting "min" or "max" arguments overly compelling either, since we already have a Math.min() and Math.max() function to handle that use-case. Perhaps it might be useful to dynamically omit the min or max side based on some condition, but that's already possible with Infinity, e.g. Math.clamp(value, 0, condition ? 360 : Infinity).)

@Richienb
Copy link
Collaborator

(I also don't find the use-case of omitting "min" or "max" arguments overly compelling either, since we already have a Math.min() and Math.max() function to handle that use-case. Perhaps it might be useful to dynamically omit the min or max side based on some condition, but that's already possible with Infinity, e.g. Math.clamp(value, 0, condition ? 360 : Infinity).)

The use case is to decrease cognitive load.

Before:

// Clamp score to a lower bound of 0
Math.max(score, 0);

After:

math.clamp(score, {min: 0});

@hax
Copy link
Member Author

hax commented Jan 18, 2024

you'd still be able to easily figure out what the order is by reading the code.

While carefully reading the code can help understand the purpose of each parameter, I still believe that having all parameters as variables can be problematic during code reviews, especially when there is a mismatch between the intention and the actual invocation (i.e., how can you tell if a code order is actually incorrect?).

Furthermore, since the motivation stated in the README is to emulate CSS clamp, using a parameter order different from CSS clamp could potentially be confusing.

If we completely disregard this point, I speculate that most people would prefer to have the value parameter in the first position. However, even with that, I always strive to minimize such cognitive overhead.

@Richienb
Copy link
Collaborator

Richienb commented Jan 18, 2024

Review:

Method Only specify max Unambiguous order Easily extendable to BigInt Conventional
number.clamp(min, max) 🟡 ✔️ ✔️ 🟡 Number prototype
number.clamp({min, max}) ✔️ ✔️ ✔️ 🟡 Option bag, number prototype
Math.clamp(number, {min, max}) ✔️ ✔️ 🟡 Option bag
Math.clamp(number, min, max) 🟡 ✔️
Math.clamp(min, number, max) 🟡 ✔️✔️ CSS signature
Math.clamp(number, min, max), Math.clampMin(number, min), Math.clampMax(number, max) ✔️ 🟡 Number prototype, but string.trim/trimStart/trimEnd
number.clamp(min, max), number.clampMin(min), number.clampMax(max) ✔️ ✔️ ✔️ 🟡 Number prototype, but string.trim/trimStart/trimEnd

Other consideration: Math.constrain alternate name.

@hax
Copy link
Member Author

hax commented Jan 18, 2024

Using {min, max} as an option bag does have a drawback in the context of the Math.xxx methods. Although option bags are not rare in the language's standard library, they are not prevalent in the Math.xxx methods specifically.

On the other hand, introducing Number.prototype.clamp also goes against the convention in some sense since all the existing mathematical functions are under the Math object rather than Number.prototype.

To me, both approaches break conventions in their own way. However, I personally lean towards Number.prototype.clamp because value.clamp(min, max) is more concise and simpler to write compared to Math.clamp(value, { min, max }).

Additionally, instance methods are easily expandable to bigInt.clamp(), whereas it would be a question whether Math.clamp would support bigint since none of the Math methods currently do. Although some TC39 representatives have expressed a desire to extend the Math methods to support bigint, I anticipate that this is a challenging and potentially impractical problem. Considering future additions like Decimal, in the long run, it might become unavoidable to move the computational methods to the prototypes.

@Richienb
Copy link
Collaborator

@hax I have revised it into a table

It seems that the most unconventional ones are the most useful
Are you familiar with how this sort of trade-off is made?

@hax
Copy link
Member Author

hax commented Jan 18, 2024

@Richienb

To be honest, each TC39 representatives are always different, making it difficult to determine which trade-off will ultimately be accepted. Sometimes, the outcome relies on the determination of the champion and the strength of opposition. Therefore, based on my observations, the final form of a proposal can sometimes be somewhat arbitrary. 🤪

@Richienb
Copy link
Collaborator

I just added number.clamp/clampMin/clampMax to the table, and it seems like it covers everything...

@Richienb
Copy link
Collaborator

Richienb commented Apr 6, 2024

Virtually all userland implementations order them like (number, min, max) which makes me think that in practice, order ambiguity might not be as big of a problem if we just order them like that as well.

@theScottyJam
Copy link

I'm going to retract this statement I said earlier:

(I also don't find the use-case of omitting "min" or "max" arguments overly compelling either, since we already have a Math.min() and Math.max() function to handle that use-case. Perhaps it might be useful to dynamically omit the min or max side based on some condition, but that's already possible with Infinity, e.g. Math.clamp(value, 0, condition ? 360 : Infinity).)

I've gotten pretty used to reading and writing code that uses Math.min() and Math.max() to handle clamping, and I don't have any practice with a clamp() method yet, so it took me a while to come around to it, but I can see now how it's easier to read understand code that's uses an explicit clamp() function with some arguments omitted as opposed to using min() or max() for clamping. With a clamp function, regardless of how the final function signature looks like, it's pretty easy to read it and picture "this parameter is the lower bound while this one is the upper bound", while with something like Math.max(n, 10) it does take me a moment to figure out if the 10 is acting like a lower or upper bound - I basically have to run an example scenario through my head to remember how it works (I'm taking the max of these two numbers, so if n is, say, 20, then the max of 20 and 10 would be 20, which means the 10 would be operating as a lower bound).

@ljharb
Copy link
Member

ljharb commented Oct 10, 2024

It's an interesting idea (and would obviate #11) to have Number.prototype.clamp and BigInt.prototype.clamp.

@michaelficarra
Copy link
Member

I claim that CSS parameter order (lower bound, number to clamp, upper bound) is the only generally intuitive order. What I mean by that is the order that someone would guess the parameters to be in with no other context or programming experience. To my knowledge, across cultures, even those using right-to-left writing systems, real numbers are depicted on a number line going from low numbers on the left to high numbers on the right. Using the number line as intuition, it makes sense that you would pass clamp a low bound on the left and a high bound on the right, with the number that is supposed to be between them... between them.

Failing agreement on that, I think we'd have to end up going with the prototype method, which again, obviously would take the lower bound on the left and the upper bound on the right.

@Richienb
Copy link
Collaborator

Richienb commented Feb 11, 2025

@michaelficarra I noticed that despite your "intuitive" order being a part of the spec for some time, every single commenter used (num, min, max) order anyways. Not a single person noticed that the order was actually (min, num, max), even though it was. So, I changed it to the one they were using.

@theScottyJam
Copy link

The number-line example does fall apart when you consider the fact that the value you're trying to clamp isn't actually clamped yet. It could be smaller than the minimum number or larger than the maximum number. It'll only be between the two numbers after the clamping operation has finished.

There are other intuitive reasons one would expect the value being clamped to come first. And I think it's fair to use a programmer's intution here since we are programmers.

When we write static methods, we typically place the subject of the method as the first argument. Some examples:

  • Object.hasOwn(user, 'name')
  • Object.assign(user, userChanges)
  • Array.from(iterator, mapFn)

There's a really strong presedence for doing this sort of thing. Clamping isn't all that different in this regard - the value we're trying to clamp would come first, and then other parameters explaining how the clamping operation should be performed would come afterwards.

That being said, I see what you're saying and understand why you feel it is intuitive. But there's other "intuitive" ways to look at this issue.

@bakkot
Copy link

bakkot commented Feb 11, 2025

The natural-language interpretation of this function is "clamp x between y and z". That is, we're operating on x. That is much much more relevant than thinking about a number line. That is why every single programming language with a three-argument clamp uses the (num, min, max) form. We are not going to break precedent with every other language.

@Richienb
Copy link
Collaborator

Updated comparison, after ruling out option bags and multiple functions from #4 (comment)

Method Unambiguous order Easily extendable to BigInt Conventional
number.clamp(min, max) ✔️ ✔️ 🟡 Number prototype, diverges from Math.min/max
Math.clamp(number, min, max) ✔️ ✔️
Math.clamp(min, number, max) ✔️ ✔️✔️ CSS signature

@Richienb
Copy link
Collaborator

I believe the argument for (number, min, max) vs (min, number, max) is: how much would we be willing to trade off consistency within the language with conventions across most other languages?

@bakkot
Copy link

bakkot commented Feb 20, 2025

One of the people who originally added the CSS function has said that with hindsight the weight of precedent suggested other order, CSS functions aren't much like JS functions anyway, and we should just do the obvious thing in JS i.e. (val, min, max).

We should not be willing to break precedent with every other language just to match CSS here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants