cjlotz / Xamarin.Plugins

Cross platform Xamarin Plugins
MIT License
113 stars 56 forks source link

Optimization: Removing Multiple Interfaces #2

Closed jamesmontemagno closed 9 years ago

jamesmontemagno commented 9 years ago

Ideally I would say that this could be 3 plugins... 1 for each Interface to be honest such as:

Phone Call Plugin for Xamarin SMS Plugin for Xamarin E-mail Plugin for Xamarin

However if keep them all in 1 Messaging plugin for Xamarin (which I think is alright) I would just have 1 interface. There is no real need from what I see to have 3.

You could just have IMessaging and that is it with all of them. Seems to add complexity for using the plugin.

cjlotz commented 9 years ago

I thought about the 3 different plugins, but I these plugins are really small wrappers around native functionality and it just felt like too much bloat to manage them as separate so I decided to scope it more around the "Messaging" level. I'll have a look at combining the interfaces into a single IMessaging abstraction. My initial design had something similar. Thanks for the comment.

cjlotz commented 9 years ago

Had a look at the single IMessaging interface again. Not too keen to go that route. My thoughts are that the MessagingPlugin container essentially already fulfills this single API role. It is the single API that you use to get access to the e-mail, sms and phone tasks. It also affords me the opportunity to inject any state required (like the current Activity, UIViewController) via the IMessagingContext construct and not worry about maintaining/resetting this state via Init methods as suggested in #3.

I also don't want to collapse the task API's into a single IMessaging API as that would leave the IMessaging API implementing too many responsibilities IMO.

Back to the question if these separate responsibilities warrant separate plugins? If the task API's where more involved I would argue yes, but due to the fact the task these API's are so thin, I find it convenient to have a single Plugin that gives me access to these 3 core messaging concepts on a mobile device (email, sms and phone call). I can choose which of these messaging constructs to use by invoking the relevant method on the MessagingPlugin, passing in the required platform state and get back a task-specific API that I can use and discard about after I've send the e-mail, sms or made the call. Thoughts?

jamesmontemagno commented 9 years ago

As mentioned I don't mind them all being in the same plugin as they provide similar functionality, I would have broken them apart since they are all used different form a different API. I think my main mention though with #3 is that you can not use your API from shared code or from a portable class library at all.

If you do a file -> new forms app and want to call:

var task = new MessagingPlugin.EmailMessenger(new MessagingContext(this));

This can't be done because of the MessagingContext. In theory you are actually just creating an Init, but now limiting the plugin to platform specific code.

I would actually drop the Messaging Context completely and do:

MessagingPlugin.Email.Method1(); MessagingPlugin.SmsMessenger.Method();

Then I would use the application context and the sharedapplication uirootview. This would make it fully cross platform in the spirit of plugins.

Does this makes sense, a bit hard to explain via typing.

cjlotz commented 9 years ago

Yes, It makes sense. If the application context and sharedapplication uirootview is enough then the issue disappears, but I had some issues with using that - I'll test it again in the sample apps. W.r.t first calling Init and then the separate task api, this is obviously not ideal from an API discoverability point of view - especially because the one call happens from the platform specific library and the other from the PCL (ViewModel etc.). I'll investigate a bit further. Frameworks like MvvmCross has the concept of an IMvxActivityMonitor that keeps track of the current activity. This allows plugins to get hold of the current Activity using the DI container. This is the cleaner solution in my opinion, but I'm not sure we are "allowed" to explore the DI container route for Xamarin plugins as it introduces additional dependencies?

jamesmontemagno commented 9 years ago

So, before launching the plugin idea I had thought about the DI container and all that jazz, but that is a whole other can of worms to open up really.

Since we are keeping it clean and simple as possible to not introduce any additional dependencies the Init() is the best way to do it in my opinion of for some reason the uirootview and app context is not enough.

cjlotz commented 9 years ago

The only issue I have with the Init method is that it happens to early. I want to get the current Activity when the API is used, not when the app starts up. I also don't want to keep track of the Activity in the plugin as that implies state the plugin needs to manage. One alternative is to make the Init take in a Func or ICallback interface which the Plugin can invoke when required to get the current Activity or UIViewController. The Init method would be initialized with the callback and use it whenever it requires the platform specific context.

jamesmontemagno commented 9 years ago

Correct, so what I would document is that they need to call Init() on the activity or UIViewController where they are calling it from. It doesn't have to be in startup code.

I am not saying it is ideal, but I think it should work alright. Let me test a few things out here.