w3ctag / design-principles

A small-but-growing set of design principles collected by the TAG while reviewing specifications
https://w3ctag.github.io/design-principles
178 stars 46 forks source link

New principle: Naming of factory methods #378

Closed LeaVerou closed 9 months ago

LeaVerou commented 2 years ago

This was brought up in our discussion of Response.json() in https://github.com/w3ctag/design-reviews/issues/741

We should have guidelines for naming factory methods. Currently the web platform is pretty inconsistent:

Perhaps we should have guidance that factory methods should start with create or from to make it obvious that they are factory methods? Not sure we should restrict it further, as these are not interchangeable.

Though there are also things like Date.parse(), which are fine as-is…

ptomato commented 2 years ago

For Temporal we took our from name from other factory methods of JS builtin objects, e.g. Array.from(), Object.fromEntries(), String.fromCharCode(). Object.create() is the oddball, I think.

domenic commented 2 years ago

IMO there are several categories of things here:

And then the async variants:

Guidance here should probably be integrated with https://w3ctag.github.io/design-principles/#constructors .

js-choi commented 2 years ago

In addition to the categories that @domenic mentioned, Ecma262 JavaScript also has some “coercion” functions that are constructors being called as if they were ordinary functions, without new. For example, Integer(v).

The iterator-helpers proposal also uses this pattern: Iterator is a constructor, but Iterator(o) (an ordinary function call) attempts to “coerce” the given input into an Iterator object.

silverwind commented 1 year ago

Should it not be forbidden to use the same name of an instance method (Response.prototype.json) and static methods (Response.json)?

Take for example the MDN Response document. They just omit the .prototype part of the names, so they could not acutally create two pages with their current URL scheme.

domenic commented 1 year ago

MDN is working on fixing their unfortunate URL-scheme choices from many years ago, and such unfortunate choices should not constrain web platform API design.

LeaVerou commented 1 year ago

Agreed with @domenic. Also, naming static and instance methods the same can actually be a very good practice in certain cases where a method does the same thing, the instance one operating on the instance itself, and the static one on the first parameter (though I can't think of an example of that off the top of my head).

LeaVerou commented 9 months ago

The original PR (#471) included this sentence:

An example of valid usage of a factory method is when an object is being created it also requires association with the parent object. document.createXXX() is an example of this.

I think this is actually an antipattern that we should actively discourage. This should have been Element.create({ document }), because:

  1. It’s a pattern that does not scale: what happens when you need to associate with two objects?
  2. It does not make it clear what object is created. With factory methods being static methods on a class, even when a subclass is returned, at least we know what the base class is, whereas something like document.createXXX() could return anything.
  3. It introduces an undesirable coupling between two classes when all we want is a relationship. E.g. what if in the future we want elements that are not associated with a document?
  4. It’s violates most precedent (the vast majority of factory methods on the web platform are static methods).

Furthermore, the way this is phrased is inaccurate: for DOM elements the parent object is element.parentNode, not document. Which highlights another problem with this pattern.

OTOH, things like CanvasRenderingContext2D#createXXX() seem more reasonable (though static methods would also not have been terrible), so the guidance should probably be more nuanced than "never do this".