GD-HTML5 icon indicating copy to clipboard operation
GD-HTML5 copied to clipboard

Bag in wiki

Open kutuluk opened this issue 6 years ago • 1 comments

https://github.com/GameDistribution/GD-HTML5/wiki/SDK-Implementation

bag:

nextButton.addEventListener('mouseUp', function () {
    if (typeof gdsdk !== 'undefined' && gdsdk.showBanner !== 'undefined') {
         gdsdk.showBanner();
    }
});

good:

nextButton.addEventListener('mouseUp', function () {
    if (typeof gdsdk !== 'undefined' && gdsdk.showBanner !== undefined) {
         gdsdk.showBanner();
    }
});

very good:

nextButton.addEventListener('mouseUp', function () {
    if (typeof gdsdk !== 'undefined' && gdsdk.showBanner) {
         gdsdk.showBanner();
    }
});

perfect:

nextButton.addEventListener('mouseUp', function () {
    window.gdsdk && gdsdk.showBanner && gdsdk.showBanner();
});

kutuluk avatar Jun 02 '19 15:06 kutuluk

The current version in the wiki contains broken code. It's updated to use showAd instead of the deprecated showBanner in the issue description, but the same bug is still there.

The gdsdk.showAd !== 'undefined' should at the very least say typeof gdsdk.showAd !== 'undefined' (ie. it's missing the typeof). Or then it should be testing against undefined (not the string 'undefined') like the second example in the description does.

I personally don't agree with the "perfect" solution as it is obfuscating the intent of the function. Also it requires a deeper understanding of JavaScript, as in why is the first check window.gdsdk and then the next check uses gdsdk directly. The solution might be perfect in terms of least amount of code but for a Wiki documentation that is meant to explain what you are supposed to do for people with a wide variety of technical experience I would keep the function as simple to read as possible. Experienced programmers like @kutuluk, who clearly know what they are doing, are free to refactor the example code as they see fit.

So to sum it up: I vote for the "very good" version.

qtiki avatar Jun 10 '20 20:06 qtiki