three.js icon indicating copy to clipboard operation
three.js copied to clipboard

Rename Vector3.sub to Vector3.subtract

Open manake opened this issue 2 years ago • 18 comments

All other methods other than sub use full word (like add, normalize, random, divide, negate, multiply, distance).

manake avatar Sep 08 '22 16:09 manake

That would mean subScalar() and subVectors() also needs to be renamed. TBH, I don't think it's worth changing the API in that regard since syntactically sub matches nicely to add.

Mugen87 avatar Sep 08 '22 17:09 Mugen87

My very first impression was that sub was a bit confusing to me. I was searching the documentation for full subtract. As a non-native English speaker such sub shortcut is not even clear that it is from subtract because sub means under to me.

I think names like XMLHttpRequest should be named like XmlHttpRequest to be consistent so this isn't isolated case of inconsistencies.

I think cd (change directory) or cat (concatenate) in linux are bad names and should become e.g. filesystem.concatenate to make sense.

Often developers name things like cd or cat and maybe they have technical knowledge but not psychology knowledge.

But I don't insist on this change, do as you like.

manake avatar Sep 08 '22 21:09 manake

sub means under to me.

Correct. It does.

I am inclined to agree that Vector3.sub() is a poorly-named method.

Whether it is worth changing at this point is a judgement call, but if it were my decision, I'd change the names of the three related methods.

WestLangley avatar Sep 08 '22 22:09 WestLangley

The new node material uses a short system for math operator nodes. They are frequently used all over the node material code base. If having sub() is really an issue, it would only be consistent to change these operators, too.

https://github.com/mrdoob/three.js/blob/f70ead9d2ebe8509e4e204ea514e42afe479573f/examples/jsm/nodes/shadernode/ShaderNodeBaseElements.js#L193-L196

I personally have no problem with shorter names since they are nice and compact but still understandable. Using full names only makes the code verbose and not more readable in this particular use case.

Mugen87 avatar Sep 09 '22 08:09 Mugen87

We are talking about the Vector3 class here.

You can change .multiply() to .mul() and .divide() to .div() -- if you really think the conventional terms are too verbose.

Or you can change .sub() to .subtract() to be consistent with the existing nomenclature.

I would prefer the latter.

WestLangley avatar Sep 09 '22 18:09 WestLangley

Generally, a user looks to have meaningful, yet compact names for these methods, he doesn't care if it's the full name or not. Being consistent with the nomenclature is fine, but it's also true that Three.js has verbose syntax. If this is really a thing to consider, using function name aliases (if Javascript permits), would satisfy both camps. If someone wants to use subtract he could do so, if he wants to use sub he could do so as well.

Not sure if this is something worth looking into though - I prefer Three.js to work correctly in the first place, the rest is just superficial in nature, really.

Yincognyto avatar Sep 09 '22 20:09 Yincognyto

TBH, we overthink this issue. sub() is a valid counterpart to add(). I would simply do nothing in order to avoid API changes and migration tasks.

Mugen87 avatar Sep 09 '22 20:09 Mugen87

sub() is a valid counterpart to add().

In English, no.

It's add, subtract, multiply, and divide.

WestLangley avatar Sep 09 '22 22:09 WestLangley

sub is a valid acronym of subtract. I've seen sub() functions in other libraries as well so I think it should be clear for what it stands for.

Mugen87 avatar Sep 10 '22 07:09 Mugen87

Can't we just add both sub/mul/div and subtract/multiply/divide?

LeviPesin avatar Sep 10 '22 07:09 LeviPesin

The engine usually does not provide method or function alias. This only bloats the API.

Mugen87 avatar Sep 10 '22 07:09 Mugen87

Personally, I prefer the short variants, but as a general outcome, you do realize that changing small syntax things like that forever will only make yet another bunch of Three.js code online obsolete, like breaking backward compatibility every few months with other stuff wasn't enough. Then, what if someone else wakes up one day thinking subtraction is better than both sub and subtract?

My point is, either do ALL these things once (preferably, right from the start, or even in a single shot Three.js wide) and then forget about them and just say NO to every request to change them, or don't do it at all. Making such changes in different moments only hurts Three.js and pushes back another set of users from it. The focus should be instead on solving real problems, like, for example, #3845, while keeping features intact - I mean, such things exist from 2013 and are only ligthly mentioned in the docs, without making clear to the user that you can't get the real rotation of a child of a non uniformly scaled parent at all. I understand it's complicated, but as a 3D library, Three.js should allow both. This is not meant to criticize, but to illustrate how a real problem looks like - by comparison, this is just adjusting the length of the pants you're wearing.

P.S. To put it in one phrase, subtract is the correct way, while sub is the convenient way. Some prefer one, others the other, and this will always be the case, whether you make the change or not.

Yincognyto avatar Sep 10 '22 10:09 Yincognyto

To an average user, like me, whatever is easier to find and remember is more convenient. And the first choice to look for is a phrase subtract. sub is a secondary word/derivative that wouldn't exist if subtract word (primary/root) didn't exist in the first place.

TBH, that idea with aliases is actually good (even if it looks like pollution and unnecessary overcomplication). But only 1 word would be in the main package and users could create their own sub alias if they would want. They can load the library and create aliases.js and do Vector3.sub = Vector3.subtract and use sub. Looks the most logical to me.

If I were designing linux commnads I would do them like this (hierarchical categorization and meaningful, easy to remember):

FileSystem.concatenate (cat)
FileSystem.makeDirectory (mkdir)
FileSystem.remove (rm)

etc.

And if some advanced user would be using FileSystem.concatenate often they could create their own cryptic alias cat (that only they would understand) or even load pre-defined aliases cat, rm, mkdir.

manake avatar Sep 10 '22 12:09 manake

Yeah, the problem with aliases is that's it's not even a Three.js specific issue, it's a Javascript one - see this SO question for details. Thinking about it, I'm not even sure if that would require changing anything in Three.js, one could simply attempt to do:

  plane = new THREE.Plane();
  ...
  var sFNACP = plane.setFromNormalAndCoplanarPoint;
  sFNACP.call(plane, object.getWorldDirection(new THREE.Vector3()), object.getWorldPosition(new THREE.Vector3()));

or

  plane = new THREE.Plane();
  ...
  var sFNACP = plane.setFromNormalAndCoplanarPoint.bind(plane);
  sFNACP(object.getWorldDirection(new THREE.Vector3()), object.getWorldPosition(new THREE.Vector3()));

instead of:

  plane = new THREE.Plane();
  ...
  plane.setFromNormalAndCoplanarPoint(object.getWorldDirection(new THREE.Vector3()), object.getWorldPosition(new THREE.Vector3()));

in the Javascript code, for example (I chose this method instead of sub vs subtract to give a better example of verbose syntax in Three.js).

That being said, this will cause the user to write more code instead of making things easy to switch between verbose and compact mode, so it doesn't really achieve the objective of simplifying things.

Yincognyto avatar Sep 10 '22 12:09 Yincognyto

@mrdoob tldr;

For consistent nomenclature in the Vector3 class, either use

add(), subtract(), multiply(), divide(),

or short forms:

add(), sub(), mul(), div(),

but do not mix them.

WestLangley avatar Sep 10 '22 15:09 WestLangley

(With inclination towards subtract. It's not "verbose". It's "default". sub is an arbitrary shortcut, not a default word.)

manake avatar Sep 10 '22 16:09 manake

Also, @mrdoob, make sure these changes, if any:

  • are operated everywhere (see above post by @Mugen87)
  • are done once and for good

Yincognyto avatar Sep 10 '22 16:09 Yincognyto

@manake Currently in Three.js, the "default" is sub, and it has less letters than subtract, hence the latter being more "verbose".

It isn't an actual word in human English language, but that doesn't matter much in computer languages, where the devs have the last say and they don't have to necessarily conform to the constraints of the human language. Otherwise, Javascript would have replaced their similar substr with substring, parseInt with parseInteger, min with minimum, max with maximum and so on. Obviously, they didn't, since they value background compatibility more, and it was the right call.

I'm not against the suggestion itself. I'm against such suggestions after lots of people got used to them. I'm against such suggestions not being handled all at once and in a final manner. I'm against such suggestions taking a pass instead of more important ones related to functionality. That's all.

Yincognyto avatar Sep 10 '22 16:09 Yincognyto