AltBeacon / android-beacon-library-reference

A reference application for the Android Beacon Library
Apache License 2.0
273 stars 186 forks source link

Create a 101 sample that uses minimum code to demonstrate the minimum functions of monitoring a beacon region. #53

Open TonyTangAndroid opened 5 years ago

TonyTangAndroid commented 5 years ago

Currently, this reference app is NOT in good state of instructing library users to integrate such library. For example, we put so many code in Application class, which is definitely a code smell. And in Application class, it references to Activity code, which is also not good. And in Application class, it has so many commented out code that serves nothing but more confusion.

With all being said, I would like to break down the huge sample app into several smaller ones that gradually covers all the features. And eventually, we could even delete the huge samples app so that every one will be able to easily focus on whatever features that they are interested in small samples.

To start this, I am to create a 101 sample app that uses minimum code to demonstrate the minimum functions of monitoring a beacon region.

davidgyoung commented 5 years ago

A few thoughts here...

I think we have to err on the side of simplicity and very small number of code artifacts to show the user how to use the library. Right now we basically have three artifacts: one Application class and two Activity classes. I would like to keep it that small.

While the inclusion of lots of code directly in an Application class is certainly not a best practice, the focus of the reference app is to simply show the user how to use the library and not to teach best practices.

The optional foreground service is what drives a lot of the code that is commented out. I agree this is distracting. Perhaps an easy way to rectify that is to move the code to its own method setupForegroundService() and leave one line that can be uncommented to do that.

I think at this point it might make sense to have at most two variants of the reference app. One could be for foreground detections (not using RegionBoostrap and with all code in an Activity) and another using RegionBoostrap in an Application class with an (optional) foreground service.

One question, @TonyTangAndroid, would you still recommend including multiple reference apps in the library repo? I'm a bit worried about the churn on these apps overwhelming the core activity coding the library.

TonyTangAndroid commented 5 years ago

If you are talking about migrating this sample app repo into the library repo, I am totally fine to remain it the same way. Because we could still refer the library source code directly as the pull request demonstrates. And if we introduce several other sample apps, this push won't be that necessary.

For now, what's in my mind that I really need to get off my chest is to write several simple samples app that demonstrate my understanding about beacon conception on what it could do and how to do it. And It will be great that you could review such samples and let me know your thoughts. I will create pull requests to here. But you do NOT have to merge it. Its main purpose is to provide a forum for us to discuss the best practice of AltBeacon library.

I personally have been struggled a lot of times on integrating and maintaining beacon features into our prod app and it has been not stable as I would hope to( More details could be provided if you are interested). I had read all the document related AltBeacon, watched all videos in youtube related to AltBeacon and blogs from Radius Network. And I am still not 100% sure that I am doing thing in the correct way. Hence, the ultimate reason to write these sample apps is to provide a way for you to verify every bit of thoughts, which, I believe, will also be helpful for other developers.