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

Rename getAll* methods with get* #40

Closed
jracle opened this issue Sep 22, 2014 · 7 comments
Closed

Rename getAll* methods with get* #40

jracle opened this issue Sep 22, 2014 · 7 comments

Comments

@jracle
Copy link

jracle commented Sep 22, 2014

My feeling is that 'All' in getAll* methods is overkill.

I would then replace all of getAll* by get*, e.g.:

  • BluetoothDevice.getServices
  • BluetoothGATTService.getCharacteristics
  • BluetoothGATTService.getIncludedServices
  • BluetoothGATTCharacteristic.getDescriptors

This would also be aligned with most existing native APIs (e.g. Android)

@jyasskin
Copy link
Member

Thanks for filing the standalone issue!

@mounirlamouri @marcoscaceres As the web-standard experts here, do you have an opinion on this naming question? We have getService to return a single service. If we use getServices to return a sequence, that only differs by one letter, so might be harder than getAllServices to distinguish when reading code.

@jracle
Copy link
Author

jracle commented Sep 23, 2014

You're welcome!

Got your point, I'll let the experts speak then. Please be aware we might also add Primary to the name, i.e. getPrimaryService(s) (see issue #41 ).

Cheers.

@marcoscaceres
Copy link
Contributor

It feels to me that getService() might be a premature optimization. How expensive is the lookup on getService() vs getServices()? How common will it be to use one over the other.

@jyasskin
Copy link
Member

getService() is more a readability optimization, although it potentially does help performance. Sample code:

my_device.getServices().then(services => {
  // Longer if Array.find() or arrow functions aren't implemented yet.
  return services.find(service => service.uuid === desiredUuid);
}).then(realCode);

becomes

my_device.getService(desiredUuid).then(realCode);

The performance difference may not exist if the UA discovers the full service tree before returning the device. If the UA discovers things lazily, the difference depends on how many services a device defines and how many a webpage uses. In the case that makes getService look best, the webpage uses 1 service, and the device exposes lots. Then getService performs one radio round-trip, while getServices performs about 1 round trip for every ~3 services. Characteristic lookups may wind up with an extra round-trip or two per result if we decide to make certain attributes non-Promises.

The case that makes getService look worst is when the the developer uses getService to open every service the device exposes, and makes these calls too far apart for the UA to coalesce requests. Then getService can actually wind up using more round-trips than getServices.

I suspect getService will be far more common than getServices because

  1. most devices expose one instance of the relevant service, and
  2. it's more convenient to pass the Promise representing a single service down to the site subsystem that uses it, than to search an array for the relevant services in order to pass them out to the subsystems later.

@jracle
Copy link
Author

jracle commented Sep 30, 2014

Thanks @marcoscaceres and @jyasskin for your replies! Though we're mixing another issue there (see #39). Sorry to have created 2 separate issues. This one is more about naming, hence for what is about function signatures I'll reply in #39.

@jyasskin
Copy link
Member

jyasskin commented Oct 1, 2014

After some discussion, I think we should take this change.

@jracle
Copy link
Author

jracle commented Oct 4, 2014

Thanks @jyasskin !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants