Final variables fix

[Defcon0]

Fixed https://github.com/admob-plus/admob-plus/issues/35

[ratson]

@Defcon0 Thank you for the PR. You can remove cordova-admob-sdk from dependencies, which is not needed for this plugin. The error is likely because you are using an old Cordova version v7.1.0, please try to upgrade to v8.1.2 first and see if the problem remains. I have no plan to support older version of Cordova yet.

[Defcon0]

The error mainly was that you used local variables in a closure (Runnable) which isn‘t allowed in Java. You can only use them uf you make them final. You even named the adUnitId „finalAdUnitId“ but didn’t made it final in the Interstitial class.

So I think this has nothing to do with the cordova version. In fact I‘m surprised you got this code compiled without errors.

[ratson]

@Defcon0 I could build without problem, it is could be either problem of the project setup or difference between my and your environment. If you could setup a demo project of the problem, then I could confirm the cause.

[Defcon0]

OK, this is very strange. To my knowledge, one can't use non-final variables in inner classes. But my knowledge doesn't need to be necessarily the latest, since I programmed Java before some years the last time.

Indeed you began doing it this way like here with finalAdUnitID but not proceed the same with callbackContext (BannerExecutor.java):

public boolean show(JSONArray args, CallbackContext callbackContext) {
        JSONObject opts = args.optJSONObject(0);
        String adUnitID = opts.optString("adUnitID");

        String finalAdUnitID = adUnitID;
        plugin.cordova.getActivity().runOnUiThread(new Runnable() {
            @Override
            public void run() {
                showBanner(finalAdUnitID);

                PluginResult result = new PluginResult(PluginResult.Status.OK, "");
                callbackContext.sendPluginResult(result);
            }
        });

        return true;
    }

But anyway, you're right that the code compiles with cordova 8.1 and a default cordova project (although I don't quite understand why ;-) ). Maybe it's because of the Java version I use (I'm currently on a different version than the one I get the errors). Here I have

java version "1.8.0_191"
Java(TM) SE Runtime Environment (build 1.8.0_191-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.191-b12, mixed mode)

What version do you have?

[ratson]

Mine is

java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

Could be the compiler get smart enough to do it for you.

[Defcon0]

But, in any case it would be better to have valid code instead of making the compiler fix the issues, no :-)? Could be that there are also other ones having this issue but yet not complaining about it like me ;-)

To be serious, merging my pull request won't hurt your code and I can install your plugin without issues (as others might, as well).

Thanks in advance!

[ratson]

It is unclear what cause your problem, the "smart compiler tricks" is just a guess at this point. It is always better to know the cause before fixing it, otherwise it is just adding more "no one understand why" code for future maintenance. That's also why I was asking a reproducible demo, it may suggest different fix.

[Defcon0]

OK, I'll investigate a little further, when I'm at home. Although I think, the cause of the problem is quite clear if I read the error message:

/home/username/customers/acme/app_name/app/platforms/android/src/admob/plugin/interstitial/InterstitialExecutor.java:41: error: local variable callbackContext is accessed from within inner class; needs to be declared final
                callbackContext.sendPluginResult(result);

Anyway: Big thanks for the plugin, it's a great job!!

[Defcon0]

Here at home I also have:

java version "1.8.0_191" Java(TM) SE Runtime Environment (build 1.8.0_191-b12) Java HotSpot(TM) 64-Bit Server VM (build 25.191-b12, mixed mode)

So it seems to be related to Cordova.

[ratson]

Closing this as nothing is actionable, added FAQ for people hit this.

[Defcon0]

Mhm, not quite happy with that. What do you say concerning https://github.com/admob-plus/admob-plus/pull/37#issuecomment-441078917 ? Why did you start taking the problem into account and didn't finish it?

[ratson]

Because it is fixed in latest Cordova, I prefer people upgrade their tooling, as I stated in README

AdMob Plus is the successor of cordova-plugin-admob-free, which provides a cleaner API and build with modern tools.

[Defcon0]

OK, clear ;-)

[adilix]

Hi ratson,

thanks for this plugin, I have this same problem even with the latest Cordova build

cordova -v
8.1.2 (cordova-lib@8.1.1)

You still think it's related to that ?