plus_plugins icon indicating copy to clipboard operation
plus_plugins copied to clipboard

[share]: make Share an instantiable object

Open cedvdb opened this issue 1 year ago • 3 comments

Plugin

share_plus

Use case

Currently every method on share are static but they do not need to be. This is less convenient for testing and ioc.

Proposal

Make Share a real object by having its method as instance methods.

cedvdb avatar Apr 05 '24 13:04 cedvdb

I personally don't see the advantage.

My approach with all 3rd party code, specially when it talks to platform implementations, is to always wrap it in a service class or similar you 100% control. That way, I don't have to worry about static methods, singletons, etc. or possible api changes.

Looking at the other plugins implementations, some allow instances (like DeviceInfo or NetworkInfo) while some use static methods as in Share (like PackageInfo).

At the end of the day, all of them use an internal static singleton, with a factory constructor, e.g.

https://github.com/fluttercommunity/plus_plugins/blob/58596acafa27ee258925e3a8a15493d13ad94995/packages/connectivity_plus/connectivity_plus/lib/connectivity_plus.dart#L24-L27

So you may have multiple instances, at the end it is the same singleton.

I'll be open to discuss further, but I don't think it is a priority for us to make this breaking change.

miquelbeltran avatar Apr 05 '24 15:04 miquelbeltran

My approach with all 3rd party code, specially when it talks to platform implementations, is to always wrap it in a service class or similar you 100% control.

I'm not sure what makes you think I'm not already doing that. However, if I want to test the wrapper, I need to pass each static function in the constructor, which is less convenient than just passing an object. Singletons are also an anti pattern.

I'll be open to discuss further, but I don't think it is a priority for us to make this breaking change.

This is just a "would have been nice to have" for me, no biggie.

cedvdb avatar Apr 05 '24 22:04 cedvdb

Singletons are also an anti pattern.

Sure, but we are talking about a plugin that talks to a platform implementation, not a typical OOP use-case. At the end, the plugin implementation uses a singleton internally.

Something that we could do, is to make all plugins accessible via a .instance static property. e.g. Share.instance.share(). That way, share() won't be a static method, you could still mock the Share class directly, and you can still access it without having to create an instance of the Share class, making the Share constructor private. I think this pattern is found in all the Flutter Fire plugins.

This is something that could be done with the upcoming Share cleanup: https://github.com/fluttercommunity/plus_plugins/issues/2819

miquelbeltran avatar Apr 06 '24 06:04 miquelbeltran