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

.get and set() pattern #49

Closed
marcoscaceres opened this issue Nov 10, 2014 · 11 comments
Closed

.get and set() pattern #49

marcoscaceres opened this issue Nov 10, 2014 · 11 comments

Comments

@marcoscaceres
Copy link
Member

Please avoid the .get() and .set() pattern. If you don't have a predefined set of properties, it would be more extensible to just have:

Vehicle.request("speed").then( (result) => { /*process result*/ }); 
Vehicle.set("speed", {/*some values*/}).then(...);
Vehicle.request(["speed", "whatever"]).then(...)
@tripzero
Copy link
Contributor

We do have a predefined set of properties. vehicle.vehicleSpeed is defined in the data spec.

@marcoscaceres
Copy link
Member Author

Sure, but that doesn't make sense and it's not extensible (having to write vehicle.vehicle... is already a code smell that something is wrong in the API design). It's much easier to grow the list of supported properties rather than the set of attributes on an object.

@marcoscaceres
Copy link
Member Author

(also, hopefully you can get rid of the data spec... that seems really unnecessary - no other spec on the Web needs such a thing, so again that is a strong indicator that something is wrong with the design of the API here.)

@tripzero
Copy link
Contributor

also, hopefully you can get rid of the data spec... that seems really unnecessary - no other spec on the Web needs such a thing

ad populum? Many specs do define data, units, etc (ie http://dev.w3.org/geo/api/spec-source.html#speed). Are you referring to the data spec as a separate document being unnecessary? No, it doesn't appear any Web specs do this. However, I am unaware of any specs that define the quantity of data that we do.

It's much easier to grow the list of supported properties rather than the set of attributes on an object.

This contradicts the statement that it's not extensible. It is extensible, just not in you opinion as easily as using magical strings. The idea, perhaps flawed, perhaps not, is that this API will use existing javascript features of introspection in addition to the availability API to discover what is available instead of relying on other obscure methods that returns a list of magical strings.

I see this as a matter of subjective preference. The technical functionality is identical.

@marcoscaceres
Copy link
Member Author

Are you referring to the data spec as a separate document being unnecessary? No, it doesn't appear any Web specs do this. However, I am unaware of any specs that define the quantity of data that we do.

Again, you need to wonder why you are defining so much stuff? It seems like the data API is defining as many interfaces as there are already in the core of the Web! That rings a bunch of alarm bells in my mind that something is not right. Do you need actual WebIDL interfaces for what is effectively data? Shouldn't those data objects just be dictionaries instead? I don't know the answer as I just started looking, just that something doesn't feel right.

I see this as a matter of subjective preference. The technical functionality is identical.

I would recommend doing a round of review through public-script-coord@w3.org about the API design (it's the coordination group between W3C and TC39). That group is specifically tasked with reviewing APIs to check how well they fit into JS usage patterns.

@tripzero
Copy link
Contributor

Again, you need to wonder why you are defining so much stuff?

vehicle.get("foo").then(...)
vehicle["foo"].get().then(...)
or
vehicle.foo.get().then(...)

What is "foo"? What units? Unless it is defined somehow, how is the developer supposed to know what to expect?

I would recommend doing a round of review through public-script-coord@w3.org about the API design

That's a good suggestion. Thanks.

@marcoscaceres
Copy link
Member Author

What is "foo"? What units? Unless it is defined somehow, how is the developer supposed to know what to expect?

It's still defined an enum. So:

enum VehicleProperties{
   "speed",
   "otherthing"
}

Of course, then the data that you get back is defined in the Spec. But what I'm saying that this doesn't need to be defined as WebIDL interfaces. It could be defined as dictionaries:

dictionary VehicleSpeed{
    short speed;
}

Where, speed is always on kilometers per hour.

@tripzero
Copy link
Contributor

I see. This is a dictionary vs. interface question. I'm not privy to why you would use one over the other. Could you make the same argument to the geolocation spec: that Coordinate should be a dictionary rather than a interface?

http://dev.w3.org/geo/api/spec-source.html#coordinates_interface

@marcoscaceres
Copy link
Member Author

Absolutely! Geolocation is not a particularly good spec (it has a ton of problems and it's grossly out of date). I would not recommend looking at it.

@marcoscaceres
Copy link
Member Author

I'm not privy to why you would use one over the other.

With regards to interfaces vs dictionary: I guess the easiest thing to ask is if the object is going to have some kind of backing implementation (in, say, c++) and if the object changes or performs some form of IPC or needs special methods? If the answer is "no" (i.e., it's just data), then a dictionary is probably sufficient.

@marcoscaceres
Copy link
Member Author

The webIDL spec (although admittedly a long, difficult, and boring read) does provide some guidance on the matter of which to use and when: https://heycam.github.io/webidl/

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

3 participants