Three.js: The pattern of parameter object / fake keyword arguments

[EliasHasle]

Description of the problem

(The Python code in this post is untested and may contain slight errors.)

Background: Python keyword arguments

Python functions can be defined with keyword arguments that have default values:

def f(x=0,y=0): return x+y

and called with f(x=3, y=2).

They can also be defined with a varargs pattern:

def f(**kwargs): x = kwargs["x"] y = kwargs["y"] #No checks for other (invalid) args in this example, and no defaults. return x+y

Here, kwargs is a Python dictionary, roughly resembling a JavaScript literal object.

The second function can be called just like the first one. Both can also be called with f(**{"x": 3, "x": 2}), which is very similar to the pattern used in three.js.

The following is a robust version with default values and error on invalid arguments:

def f(**kwargs): x = kwargs.pop("x", 0) y = kwargs.pop("y", 0) if len(kwargs) > 0: raise Error("Invalid keyword arguments: %s" % (str(kwargs))) return x+y

Many classes take an object as constructor parameter, which gives similar benefits to Python's keyword arguments:

1. Order is flexible (no need to remember or check docs every time).
2. Optional parameters can be omitted without passing undefined.
3. The calling code is more informative of the purposes, because it must include the names.

It likewise shares some disadvantages with Python's kwargs:

1. The calling code is longer than with positional args.
2. Instead of remembering/checking parameter order, one may need to remember/check parameter names (which arguably is easier).

On top of this, it has some disadvantages that Python's kwargs don't have:

1. Inline default is for the whole object. This can be alleviated by using deconstruction on the left-hand side of the default value assignment, but that does not make very readable code. The style in three.js seems to be to handle the parameters very carefully in the constructor body, adding defaults on the way (which is a decent solution).
2. Python's kwargs supports the following pattern for "eating" kwargs, forwarding the remaining ones and finally "spitting" out the remaining:

````
class class1:
def __init__(self, arg1, **kwargs):
kwarg1 = kwargs.pop("kwarg1", "the default value")
self.arg1 = arg1
if len(kwargs) > 0:
print("Warning: Unused kwargs: %s" % (str(kwargs)))

class class2(class1):
def __init__(self, arg2, arg1 *kwargs):
self.arg2 = arg2
kwarg2 = kwargs.pop("kwarg2", "the default value")
super().__init__(self, arg1, *kwargs)
self.arg2 = arg2
````

JS, on the other hand, has no pop on objects. It may be polyfilled (get and delete, or else set to default or throw missing), but that would be kind of a hack. Or one can have a global util for it, to be able to use the same kind of pattern.

One (minor) obstacle to replciating the above Python pattern in JS is that JS object don't keep track of their number of properties. https://stackoverflow.com/questions/126100/how-to-efficiently-count-the-number-of-keys-properties-of-an-object-in-javascrip

I am sure there exist other advantages and disadvantages than the ones I have mentioned. @mrdoob at least once expressed a wish to move away from the parameter object solution, or at least reduce the use of it: https://github.com/mrdoob/three.js/pull/14726#issuecomment-414086592
He argues that it is somehow not resilient to API changes. I am not sure I understand why. Maybe the kwargs "eating and spitting" approach will be regarded as sufficiently robust, in the sense that it quickly tells the user that something is wrong in the case of an invalid (outdated) parameter name.

What are the good alternatives to the parameter object, really? I don't exactly favor the setters that mrdoob outlined, when setting many properties at once. Object.assign will obviously work in many cases, though.

[mrdoob]

Do you mind writing a TL;DR? 😇

[EliasHasle]

Hm you mean you DR, because it was TL? :-/

I did make a details/summary for Python kwargs. That is a TL;DR for that section.

TL;DR for @mrdoob : What lies in the future for constructor parameterization in Three.js? (...when our great ruler does not read a post outlining a solution that conserves the parameter object? 😉 )

[looeee]

Can you write a TL;DR for me too? I've read your post thoroughly, twice, but I can't see the solution that you're proposing. Not sure I understand what problem you want to solve either TBH

[EliasHasle]

OK. Problem: I am worried that three.js will move _away_ from using the parameter object rather than _towards_ it. I don't understand exactly what are the big problems with the parameter object pattern, and would like to know that. Also, I am proposing a general pattern to keep handling of "keyword arguments" tidy, without knowing whether that will affect the problems that @mrdoob sees with the parameter object.

The Texture class is an example of one that IMO would benefit from the parameter object pattern, because there are so many parameters that are often varied. WebGLRenderTarget takes a parameter object for Texture as extra parameter, which I find very nice.

Rather than expecting you all to read and understand Python code, I can do with some clearer JS pseudocode instead:

_eatArgument and _spitArguments may be private methods, common to all classes.

````
_eatArgument(pars, key, default=undefined, throwOnMissing=false) {
let value = default;
if (key in pars) {
value = pars[key];
delete pars[key];
} else if (throwOnMissing) {
throw Exception("Missing value for required keyword argument "+key);
}

return value

}

spitArguments(pars) {
const c = reference to current class?
let errors = ["Invalid keyword arguments received: "];
for (let key in pars) {
errors.push(key+", ");
}
let errorMsg = errors.join("");
throw Exception(errorMsg);
}
````

Non-eaten args will be passed to the superclass constructor. Only the base class will (optionally) spit arguments.

Edit: I am implementing this pattern for a private (to become public) project that uses ES classes. It turned out I could not use an EaterSpitter superclass, because this.eat(pars, keys, defaults) would have to be called before super(pars). So I had to make them external functions. On the other hand, the external functions eat/spit can also easily be used from arbitrary functions that use kwargs.

[Mugen87]

So there are three options you have mentioned, right?

1. Parameters

const texture = new Texture( image, Texture.DEFAULT_MAPPING );

1. Parameters object

const texture = new Texture( { 
    image: image
    mapping: Texture.DEFAULT_MAPPING
} );

1. Setters

const texture = new Texture();
texture.setImage( image );
texture.setMapping( Texture.DEFAULT_MAPPING );

You say, three.js should favor the usage of option 2, right?

Setting properties always means you transfer an object into a specific state. Applying ctor parameters is one way to do this but in many cases you just set parameters like so texture.image = image. The idea of just using setters is that you as a library developer have more control about that state specification. Some people think it also makes the code more readable and descriptive.

[EliasHasle]

You say, three.js should favor the usage of option 2, right?

I lean towards option 2, but I welcome arguments against it and for the other approaches.

Regarding option 3, would there be a consistent pattern so that e.g. setPosition( v ) would either do this.position.copy( v ) or this.position = v internally? I would think that assignment is clearer by simply using obj.position = v (bad example, since Object3D position is now read-only, for some reason), so that setPosition( v ) should be the former instead. Right?

Even with positional constructor parameters or parameter object, there is always room for ambiguity in this regard. Maybe even more so than with setters, if the setters follow a consistent pattern where assignment is left to the assignment operator.

Anyway, conceptually I think of the constructor as the natural place to read and apply a specification for an instance of a class. It will have to apply defaults for missing specification anyway, so that the object is a valid instance. (Being able to modify the object afterwards surely is a good bonus.) Would option 3 involve keeping some essential constructor parameters, such as image for Texture, while throwing out all the ones that make sense to modify after object creation?

[Mugen87]

Would option 3 involve keeping some essential constructor parameters, such as image for Texture, while throwing out all the ones that make sense to modify after object creation?

Well, basically yes. You assume that even without ctor parameters the object is in a valid state.

However, thinking about this issue again, this whole topic is not that urgent from my point of view. I mean it was mentioned a few times that we might change the API for new classes. But I don't think this will affect existing ones for backwards compatibility reasons. Moreover, it might be more consistent to stick to the current API design than doing something different.

[looeee]

it might be more consistent to stick to the current API design than doing something different.

That would be my preference. If it's not broken, don't fix it, and definitely don't mix two API styles in the same project.

Also, I'm struggling to see the reason for option 3, it seems like it adds a lot of extra keystrokes to instantiating objects, and hundreds of extra .setXXX methods throughout the codebase, with no benefits.

[mrdoob]

The reasoning for option 3 was to combat the lack of types in Javascript.

Say that someone does material.transperent = true and spends a few hours until they realise they made a typo. Javascript just added a property to the object so it has no reason to complain.

However, if the API was material.setTransperent( true ), then javascript will complain because that method doesn't exist.

This also happens in option 2 but Material warns when a parameter name doesn't match.

In theory Object.freeze() helps with this, but it used to affect performance.

[mrdoob]

Hm you mean you DR, because it was TL? :-/

Yeah, I was not sure why we were talking about Python.

[Mugen87]

@EliasHasle Can this issue be closed now?

[EliasHasle]

@Mugen87 I got my questions answered. I am not sure yet that no one is interested in pursuing the alternative pattern for parameter handling that I propose, though. I have shared my (experimental) implementation here: https://github.com/EliasHasle/kwargs.js

My experiences with it are mixed. It does work as expected, and complains both on missing and on invalid arguments. 👍 It turns out the pattern can be used in many ways (👍 / 👎 ), and that it is not alone a universal solution for all cases:

• Sometimes, one will assign values to some pars within a subclass constructor.
• When a "keyword argument" is itself an object, eat and spit may be applied to that too, to set requirements and defaults on properties of individual passed objects. En example could be to check whether a passed kwarg is "vector3-like", in requiring that it has properties x,y and z. If it is intended to support Vector3 objects, spit should not be called, as it will treat all other properties as invalid args.
• Setting a default value through an argument to the eat function makes most sense if the default is a simple value, because it will be a waste (and eventually garbage-collected) when the par already has a value. Therefore, when defaults are complex constructed objects, they will be assigned in a more traditional way (par = par === undefined ? new ExpensiveObject() : par). 😕
• With ES6 classes, the subclass args cannot be applied to this before the superclass constructor is called with the remaining arguments. Therefore the subclass args must be put aside in locals before the super call, and applied afterwards.
• Some points that I have forgotten right now... (There are few in the README.MD of kwargs.js)

I will continue using it for my project, as I overall find it useful. But it may not be for everyone. 🙂

[EliasHasle]

Another aspect of this is possible futures where one wants to achieve better performance with TypeScript, e.g. by making the code compilable to WebAssembly using the TypeScript subset AssemblyScript. That may perhaps be out of reach anyway, though. But if it's not, I suppose having a more static interface (with a fixed number of positional, typed, arguments to every constructor/method/function) is a necessary precondition.

[mrdoob]

Please, keep in mind that I try to read everything that gets posted here.

Brainstorming about possible futures is interesting but these discussions steal time and energy I could be investing in reviewing PRs that are pending to be merged.

I would appreciate if you could keep your posts short and to the point. Otherwise my brain will start skipping your posts.

[EliasHasle]

It was not my intention to reopen a closed discussion, only to add another perspective (mostly for future readers).

I realize now that mental overload is a legitimate concern in your position, with so many new issues and PRs every week. I will try to keep my posts shorter (Edit: Rereading my original post here, it shows more clearly to me as _not "to the point"_), maybe add more TLDRs (:smile:), and also be patient with my own PRs. :angel: Thank you.

[looeee]

@EliasHasle it's probably best to have these kind of discussions on the forum. Some great ideas can come out of open brainstorming sessions but this repo is not the right place to have them.

I would have been happy to discuss your suggestion more deeply (even if just to explain why I don't support it) but I didn't want to add to the noise here.

I've suggested that we enable the discourse linkback plugin in #16980. This would help to connect the forum and this repo more closely, which would facilitate these kind of discussions while keeping the noise here to a minimum.