CrossGeeks / GoogleClientPlugin

Google Client Plugin for Xamarin iOS and Android
https://www.pujolsluis.com/google-client-plugin-for-xamarin/
MIT License
89 stars 37 forks source link

Events are backed by a static field #47

Closed maurosampietro closed 4 years ago

maurosampietro commented 4 years ago

Currently OnLogin, OnLogout and OnError events are defined as follows:

        static EventHandler<...> _onLogin;
        public event EventHandler<...> OnLogin
        {
            add => _onLogin += value;
            remove => _onLogin -= value;
        }

I get you have a particular usage in mind when using this library.

In particular you whould like us to subscribe to OnLogin before requesting user data and you want us to unsubcribe as soon as we get the results; maybe using only an instance of GoogleClient throughout the entire app, by the way this could not be possible or may not be the programming style we are using.

In those different scenarios a static backfield causes problems. Ranging from the same handlers being called multiple times to memory leaks.

I'd suggest to remove the static keyword and turn to a simple event.

        private EventHandler<...> _onLogin;
        public event EventHandler<...> OnLogin
        {
            add => _onLogin += value;
            remove => _onLogin -= value;
        }

which leads to this

public event EventHandler<...> OnLogin;

Char0394 commented 4 years ago

Thanks for your feedback, really appreciated. We just made proper changes related to this