AndriousSolutions / ads

Other
58 stars 11 forks source link

Removed need for redeclaring bannerId #18

Closed adithyaxx closed 4 years ago

adithyaxx commented 4 years ago

Fixes #17

Andrious commented 4 years ago

@adithyaxx Excellent! I do appreciate your input. Particularly, if it produces 'production ads' and not 'test ads'. However, it does introduce redundancy as you hinted in the the issue posting.

I mean, why bother initializing the ad into memory in the first place if it's only going to be released and loaded again when it finally comes time to showing the ad?! Right?

Of course, I know why---because then it then supplies 'production ads' and not 'testing ads' and users are happy. However, as the author of this library package, I don't have the luxury of supplying such an immediate fix...when it should work in the first place. And as far as I know, it does work for other users...including myself. I need a better solution to this. I need to know why you're getting 'test ads' when you should be getting 'production ads.'

Supplying such an unnecessary change will only compound the maintenance of the code in the future. You see, your suggested change merely supplies a parameter to the function, _bannerAd.show() change

Doing so will 'dispose of' the first ad residing in memory. It will dispose of the first one and then load a second one into memory which is then shown because of the parameter, 'show: true'. See how that works?

load

Instead, this may involve a rewrite to an extent, and not load the ad when initializing in the first place---removing the redundancy.