MFlisar / GDPRDialog

GDPR fragment dialog implementation
Apache License 2.0
211 stars 53 forks source link

It would better if IGDPRCallback was not coupled to Activity #77

Closed xelnaga closed 6 years ago

xelnaga commented 6 years ago

It would better if the IGDPRCallback interface was split into separate interfaces for onConsentNeedsToBeRequested and onConsentInfoUpdate and then have these callbacks passed to checkIfNeedsToBeShown() and showDialog() as required instead of forcing them to be implemented on and tightly coupled to an Activity class.

This would make the code easier to reason about, permit the use of inline / anonymous callbacks and make it easy to allow different callback handers for different use cases (for example: dialog automatically shown versus dialog shown from user clicking in settings).

It would also be helpful for the method parameters on the callbacks to be annotated to indicate they are not null for kotlin use.

MFlisar commented 6 years ago

I don't think so. I made this this way to avoid memory leaks. The library uses DialogFragments and not simple dialogs, this means after screen rotation / activity recreation the dialog must know where to send the result to (which is the new activity of course)...

If you have a better solution, please explain further... Passing in callbacks will not work if they are placed inside an activity and will result in a memory leak... Singleton instance callbacks may work as well, but still this is just another work around...

Decoupled communication would be possible through a bus / rxjava based solution or similar, but this is not part of this library and would just unnecessarily blow up the dependencies...

Clean interface annotations for kotlin are a good idea though

xelnaga commented 6 years ago

OK, fair enough.