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

What is the deal with the Sensor "constructor"? #1

Closed
domenic opened this issue Sep 5, 2014 · 6 comments
Closed

What is the deal with the Sensor "constructor"? #1

domenic opened this issue Sep 5, 2014 · 6 comments

Comments

@domenic
Copy link

domenic commented Sep 5, 2014

It looks like it is serving a few purposes:

  • An abstract base class for the concrete temperature sensor etc. classes
  • A namespace upon which to hang objects

I think these would be best separated? E.g. a namespace object, window.sensors, in which lives SensorBase + Temperature + Proximity?

@rwaldron
Copy link
Owner

rwaldron commented Sep 5, 2014

  • An abstract base class for the concrete temperature sensor etc. classes
  • A namespace upon which to hang objects

Yes and yes.

I think these would be best separated? E.g. a namespace object, window.sensors, in which lives SensorBase + Temperature + Proximity?

Can you offer a rationale? Why lowercase sensors? There are many classes in the platform that don't allow direct construction, but are used to create instances by some other means. The difference really, is that Sensor's subclasses are defined as properties of their superclass, which is to avoid creating a handful of new global names.

@domenic
Copy link
Author

domenic commented Sep 5, 2014

Why lowercase sensors?

In my vision there is a separation between a namespace object (window.sensors) and the abstract base class (window.sensors.SensorBase).

There are many classes in the platform that don't allow direct construction, but are used to create instances by some other means.

These are bad and I am trying to kill them ^_^. Classes for which instances exist should be constructible. (Sensor is not one of those though, since no instances of Sensor exist, just instances of its subclasses. And yes, "instances" is a confusing word to use; not sure what is better.)

The difference really, is that Sensor's subclasses are defined as properties of their superclass, which is to avoid creating a handful of new global names.

I am all for avoiding the global name pollution, but I think that re-using the abstract base class as the namespace is strange. Normally static properties of a class are not other classes, but instead utility methods or constants or something.

@rwaldron
Copy link
Owner

rwaldron commented Sep 5, 2014

In my vision there is a separation between a namespace object (window.sensors) and the abstract base class (window.sensors.SensorBase).

Yes, that part was clear, but why?

And yes, "instances" is a confusing word to use; not sure what is better.)

Haha, I think it's correct—it reads correctly to me ;)

but I think that re-using the abstract base class as the namespace is strange.

No trolling, but why? I want to tease out the reason for the objection. Is just that it's not something you often (or ever) do?

Normally static properties of a class are not other classes,

Here are some pseudo-examples based on actual APIs in Johnny-Five:

// These...
var d = new Sensor.Digital(9);
var a = new Sensor.Analog(0);

// produce an instance as it would be created by otherwise doing this: 
var d = new Sensor({ pin: 9, type: "digital" });
var a = new Sensor({ pin: "A0", type: "analog" });

Or...

var proximity = new IR.Proximity("i2c-ir-proximity-device");
var reflection = new IR.Reflection("i2c-ir-reflection-device");

// Same as: 
var proximity = new IR({
  model: "i2c-ir-proximity-device"
});

var reflection = new IR({
  model: "i2c-ir-reflection-device"
});


// IR just loads in a device profile to create the instance

Or...

var nunchuk = new Wii.Nunchuk();
var classic = new Wii.Classic();

// Same as...

var nunchuk = new Wii({
  model: "RVL-004"
});

var classic = new Wii({
  model: "RVL-005"
});

// Same implementation pattern as IR: Wii loads in an appropriate device profile

Note that these examples, the static property definitions aren't subclasses, and the classes that they are attached to are directly instantiable. Arguably, the Sensor.* base class design is even better organized.

@rwaldron
Copy link
Owner

rwaldron commented Sep 5, 2014

Sensor could be instantiable, it just wouldn't do anything, which is also totally ok.

@domenic
Copy link
Author

domenic commented Sep 5, 2014

Is just that it's not something you often (or ever) do?

I guess that is it. I have a conceptual model wherein namespace objects and classes/constructors are very different types of things, and I never would have mixed them together.

Perhaps one way of thinking about it is that namespace objects are poor-man's modules. If we had modules in the browser, it would be e.g. import { Temperature, Proximity } from "web/sensors";, and SensorBase wouldn't even be exported. (You could get it via getPrototypeOf if you wanted, but there's no reason to export an abstract base class). So from this perspective I would rather stuff everything into window.sensors as a namespace object.

Another thing that I just thought of: when I see new Sensor.Temperature(...), I wonder, "what is Sensor? Why is it capitalized? It must be a class? Can I construct it?" Then I go learn that it is an abstract base class. Whereas when I see new sensors.Temperature(...), it feels similar to navigator.geolocation.getCurrentPosition()---I know that the navigator.geolocation bits are just a namespace holding the interesting thing, and not an interesting independent entity. So new Sensor.Temperature(...) kind of puts the abstract base class in your face more, instead of hiding it as an implementation detail.

I don't think your examples, where Sensor is non-abstract in Johnny-Five, change my answer too much. I think it's still a bit weird to have constructors as static properties of another constructor. The IR example in particular is strange. If I understand correctly, there is only the IR class, and in that case IR.Proximity is just a factory method for a particular way of constructing. I wouldn't do new IR.Proximity(...) but instead IR.proximity(...) (similar to Proxy.revocable(...)).

EDIT: upon re-reading I see all your examples are the same type; IR is not special. But in all of them, it seems that either: "same as..." doesn't literally mean the same result, or in one or the other case new X does not produce an instance of X.

@rwaldron
Copy link
Owner

rwaldron commented Sep 5, 2014

Perhaps one way of thinking about it is that namespace objects are poor-man's modules. If we had modules in the browser, it would be e.g. import { Temperature, Proximity } from "web/sensors";, and SensorBase wouldn't even be exported.

This is a strange point to bring into the discussion, since the browser's platform objects and APIs have never been designed this way. If this were a module, I wouldn't even bother with Sensor and just export the relevant constructors—but it's not. Despite this being strange, the argument is compelling in terms of forward looking design, so I will update accordingly.

I wouldn't do new IR.Proximity(...) but instead IR.proximity(...) (similar to Proxy.revocable(...)).

Interesting. I kept with capitalized properties to make it clear that they were also constructors instances
¯_(ツ)_/¯

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

2 participants