matomo-org / matomo-sdk-android

SDK for Android to measure your apps with Matomo. Works on Android phones, tablets, Fire TV sticks, and more!
BSD 3-Clause "New" or "Revised" License
393 stars 164 forks source link

Proposal: Get rid of Piwik/Tracker differentation #27

Closed d4rken closed 9 years ago

d4rken commented 9 years ago

I see no need for the Piwik AND Tracker class except to be able to have multiple Trackers for thread safety. What if we make the Tracker abstract & threadsafe. e.g.

class MyTracker extends PiwikTracker {
    @Override
    public String getTrackerUrl() {
        return "foo";
    }
}

Then in your code you just call MyTracker.get(getContext()).doStuff()

Would love some comments on this.

dotsbb commented 9 years ago

I believe that managing Tracker instances should be similar to the Google Analytics SDK Tracker. @mattab please confirm if it's true.

autoBindActivities is not mandatory and could be refactored.

d4rken commented 9 years ago

I understand, i didn't know that staying close to GA was desired. Or maybe i could have figured that as Piwik itself is quite similar.

I'll submit individual PR for the other changes i mentioned, excluding this Piwik/Tracker merge.

I'll look into adding a helper class to the SDK that would allow it to stay close to the GAnalytics design but still offer a few shortcuts to make it easier to use.

Hearing that confirmation from @mattab about this would be cool though.

dotsbb commented 9 years ago

@d4rken Perfect! I look forward to your PRs.

mattab commented 9 years ago

quick feedback: I'm not familiar with Android development so unfortunately I don't know much more. but the general idea is that if Google implemented it this way for their SDK, I think there would be a reason for it, maybe some complex use case that would benefit from this feature...?

d4rken commented 9 years ago

@mattab but there is no direct goal to stay close to their api design? looking at other APIs, e.g. In-App-Purchases API, they sometimes make it overly complicated.

In any case the documentation linked by @dotsbb shows a similar approach to as our SDK currently uses just sans the HashMap, while mentioned in Googles docs, it's an example how the dev could implement it, it's not there by default.

I agree that we should keep it this way so as to leave a dev the option to extend it the same way that Google proposes it in their documentation.

I'll leave this be until after a few other PRs, then circle back to here, then see if i can make it a bit simpler, but still offer a clean way to allow for multiple different Tracker instances.

Thanks for everyone input.

d4rken commented 9 years ago

I think this issue is resolved for me, we will keep the Piwik/Tracker differentiation and I will submit a PR for #23 which will furter align it with how this is setup in GoogleAnalytics.

It will allow people to make the same enhancement for different trackers that the documentation from GA suggests.