AltBeacon / android-beacon-library

Allows Android apps to interact with BLE beacons
Apache License 2.0
2.83k stars 836 forks source link

Outdated `BeaconConsumer` code sample in documentation #1036

Closed Whathecode closed 3 years ago

Whathecode commented 3 years ago

The code sample in the BeaconManager and BeaconConsumer documention seem to be outdated.

It overrides different methods than defined on the interface.

davidgyoung commented 3 years ago

@Whathecode, I agree the examples are confusing, but I do not think they are incorrect.

The examples are confusing because they both show an Android Activity instance that implements the BeaconConsumer interface. The methods shown with the @Override annotation aren't necessarily overriding BeaconConsumer but instead overriding methods the Activity base class instance.

Example methods that override Activity method: protected void onDestroy() protected void onCreate(Bundle savedInstanceState)

Example methods that override BeaconConsumer method: abstract void onBeaconServiceConnect()

Activity methods that implement BeaconConsumer methods: (these are invisible because they are part of the class definition for Activity) abstract boolean bindService(Intent intent, ServiceConnection connection, int mode) abstract Context getApplicationContext() abstract void unbindService(ServiceConnection connection)

Does this make sense?

Rather than clarify the documentation, the plan for the library is to deprecate BeaconConsumer in a future release along with BeaconManager#bind, BeaconManager#unbind, and RegionBootstrap in favor of auto-bind alternatives described in #1035 and available in the 2.19-beta3 release. These changes will simplify the API and make BeaconConsumer unnecessary.

Whathecode commented 3 years ago

Activity methods that implement BeaconConsumer methods: (these are invisible because they are part of the class definition for Activity) abstract boolean bindService(Intent intent, ServiceConnection connection, int mode) abstract Context getApplicationContext() abstract void unbindService(ServiceConnection connection)

Thanks! This is what threw me off. In our codebase these were apparently implemented manually; we do not extend from Activity. I wonder what that indicates a code smell. 🤔

davidgyoung commented 3 years ago

@Whathecode -- I would not worry about a code smell on your end -- if anything the code smell is with the design of the BeaconConsumer interface which assumes it is being used by an Android Activity or Application, which is why it has those extra methods that you have to implement if you are not using an Activity or Application. This is one of the many reasons I want to get rid of it to simplify the API.

If you want to clean up your code, you might try the new start/stop ranging and monitoring methods in #1035 / 2.19-beta3 that do not require a BeaconConsumer. (These are still beta though, so don't throw them into production without sufficient testing. If you notice any problems with them please let us know!)