search

matomo-org / matomo-sdk-ios

Matomo iOS, tvOS and macOS SDK: a Matomo tracker written in Swift
MIT License
388 stars 164 forks source link

Variable argument list of screen names #57

Closed soulchild closed 9 years ago

soulchild commented 9 years ago

Why did you opt for a variable argument list in the sendViews method instead of just an NSArray? It makes it very hard to add a category for this method because variable argument lists can not be passed down to other methods in Objective-C (at least not that I know). Just asking because I'd like to add a method which auto-escapes slashes in screen names.

By the way: Does the whole slashes-to-build-sub-views-functionality even make sense when the variable argument list (or alternatively an array) already provides a means to separate screens/build a hierarchy? I have quite a lot of screen name titles which contain slashes and it'd nice if piwik-sdk-ios could (optionally) handle those without having to escape them in my controllers.

mattab commented 9 years ago

I have quite a lot of screen name titles which contain slashes and it'd nice if piwik-sdk-ios could (optionally) handle those without having to escape them in my controllers.

sounds like a Piwik issue rather than iOS sdk, see: https://github.com/piwik/piwik/issues/2956 btw you can change the page title category delimiter in your Piwik: http://piwik.org/faq/how-to/faq_62/

soulchild commented 9 years ago

You're right! I've just started working with Piwik and didn't know about this configuration setting. I can totally understand that the iOS SDK aims at staying true to the "Piwik-way-of-things". It'd just be nice if there was a way to more easily augment PiwikTracker by providing a sendViews method which doesn't use a variable argument list. Would this be desirable? If so, I can think of something and send you guys a pull request.

mattab commented 9 years ago

I don't know about iOS dev, @mattiaslevin is the expert :+1:

mattiaslevin commented 9 years ago

As stated the Piwik server builds a view hierarchy based on slashes. The SDK is just trying to be consistent with this behaviour.

I see a couple of questions here:

1) var arg vs. array. I agree with you. It would probably be better with an NSArray and allow a category implementation using var args. Too late for that now (oh the joy of API design).

Lets see if we can fit a new method taking an NSArray argument. The var arg method should create an array and call this new method. @soulchild If you like to have a go, send me a pull request and we work something out (or I will probably implement it myself in a near release).

2) Screen view separator (currently slashes). One option would be to allow the dev to configure the separator (or absence of separator) in the SDK. I am really trying to avoid adding more configuration options in the SDK since there are too many already.

You could use - (BOOL)sendView:(NSString*)screen and build your own concat String of your view hierarchy with any separator you like (on none) in a category and call this method. Is this and option for you?

soulchild commented 9 years ago

I'll have a go at #1 and submit a pull request today. ;-)

mattiaslevin commented 9 years ago

Thanks for the pull request. I will close this ticket now.