Skip to content
This repository has been archived by the owner on May 19, 2021. It is now read-only.

[RFC] Better events #38

Closed
ratson opened this issue Dec 19, 2016 · 4 comments
Closed

[RFC] Better events #38

ratson opened this issue Dec 19, 2016 · 4 comments

Comments

@ratson
Copy link
Owner

ratson commented Dec 19, 2016

I am working on next release to provide better API usage, one area to fix the emitted events.
I would like to hear some feedback for the following proposals before proceed,

1. Generic events between different ad-types. E.g. AdLoaded, AdFailed events with adType in the event data.

Pros

  • Less event types
  • Some generic handling function, only need to listen to one event type. E.g. log AdFailed

Cons

  • Some ad-types may emit more or less events. E.g. AdRewarded is only used by rewarded video
  • Some ad-types event data may contain extra information, which can be hard to consume as adType need to be checked.

2. Each ad-type has its own set of events. E.g, BannerLoaded, InterstitialFailed events.

Pros

  • No need to check adType in the handling function

Cons

  • Increase the number of events

I prefer (2) as it is more explicit, what do you think? @becvert @vintage @code4youreal @warcry2000
Maybe you have better idea.

@vintage
Copy link
Collaborator

vintage commented Dec 19, 2016

I've been working with both approaches "generic events" and "specified events". As you said the generic events have one advantage - you can listen just for one event and receive multiple types of events using single line of code. Even if that sounds great, it's almost useless as in all of my apps I've had to do a conditional statement to check what exactly came in and decide what to do with it. I'm for the second approach as it gives more granularity and would keep the code clean.

Honestly the solution which I liked the most is the one used by the Heyzap - https://github.com/Heyzap/heyzap-cordova/blob/master/docs/advanced.md which simply split all the events into "namespaces", so we would have something like:

  • AdMob.Banner.Events.LOADED
  • AdMob.Banner.Events.CLICKED
  • AdMob.Interstitial.Events.FETCH_SUCCEED
  • AdMob.Interstitial.Events.FETCH_FAILED

@warcry2000
Copy link
Collaborator

Hello,
Since the events were so different, and because there were only a few common ones, I separated the events for each one.
So if someone uses only banner, worry about the events of the banner, if someone uses only videos, then only worry about the events of the videos.

Also I think the code would be cleaner.

@ratson
Copy link
Owner Author

ratson commented Dec 20, 2016

Awesome, I like the namespace idea, it could group the methods too.
e.g. admob.banner.events.LOADED, admob.rewardvideo.show()

@warcry2000
Copy link
Collaborator

I think it is a great idea.

@ratson ratson mentioned this issue Dec 25, 2016
@ratson ratson closed this as completed Jan 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants