As part of #158287 I've been switching our plugins over to the Kotlin Pigeon generator, and then relying on Kotlin/Java interop to leave the rest of the plugin in Java. As identified in my comments there, the lack of Java-style builders is a pain point for some conversions, and since IAP does a lot of mirroring of data classes that start on the native side, it will hit this in a significant way.
I can see three paths for switching to the Kotlin generator for this plugin:
-
Just use positional params. From a readability standpoint, this is actually fine because essentially all the object construction happens in Translator.java, with code that looks like this:
return new PlatformPurchaseHistoryRecord.Builder()
.setPurchaseTime(purchaseHistoryRecord.getPurchaseTime())
.setPurchaseToken(purchaseHistoryRecord.getPurchaseToken())
.setSignature(purchaseHistoryRecord.getSignature())
.setProducts(purchaseHistoryRecord.getProducts())
.setDeveloperPayload(purchaseHistoryRecord.getDeveloperPayload())
.setOriginalJson(purchaseHistoryRecord.getOriginalJson())
.setQuantity((long) purchaseHistoryRecord.getQuantity())
.build();
Replacing it with:
return new PlatformPurchaseHistoryRecord(
purchaseHistoryRecord.getPurchaseTime(),
purchaseHistoryRecord.getPurchaseToken(),
purchaseHistoryRecord.getSignature(),
purchaseHistoryRecord.getProducts(),
purchaseHistoryRecord.getDeveloperPayload(),
purchaseHistoryRecord.getOriginalJson(),
(long) purchaseHistoryRecord.getQuantity());
is fine for readability because the getters provide the names. My concern with doing that in this case (as we've done in small cases in other plugins) is that there's a fair amount amount of churn in the the underlying APIs (roughly every year there seem to be non-trivial changes), and while the second version is readable, it's not safe. If someone accidentally swaps the order order of two params with the same type, it will compile, and not be caught via review. Unit tests might or might not catch it depending on how thorough we are.
-
Block on adding the optional builder generation to the Pigeon Kotlin generator (which would be blocked on the in-progress ffigen/jnigen mega-PR, since we don't want to cause conflicts with that)
-
Convert just Translator.java to Kotlin, so that we have named arguments, which gives us the same reviewable safety as the builder pattern.
I'm curious what the Android team's preference is. I think 3 would be very simple, as it's a very straightforward file that Android Studio auto-conversion should handle without problems, but it would be a change from our current policy of only using Java for non-generated code.
As part of #158287 I've been switching our plugins over to the Kotlin Pigeon generator, and then relying on Kotlin/Java interop to leave the rest of the plugin in Java. As identified in my comments there, the lack of Java-style builders is a pain point for some conversions, and since IAP does a lot of mirroring of data classes that start on the native side, it will hit this in a significant way.
I can see three paths for switching to the Kotlin generator for this plugin:
Just use positional params. From a readability standpoint, this is actually fine because essentially all the object construction happens in Translator.java, with code that looks like this:
Replacing it with:
is fine for readability because the getters provide the names. My concern with doing that in this case (as we've done in small cases in other plugins) is that there's a fair amount amount of churn in the the underlying APIs (roughly every year there seem to be non-trivial changes), and while the second version is readable, it's not safe. If someone accidentally swaps the order order of two params with the same type, it will compile, and not be caught via review. Unit tests might or might not catch it depending on how thorough we are.
Block on adding the optional builder generation to the Pigeon Kotlin generator (which would be blocked on the in-progress ffigen/jnigen mega-PR, since we don't want to cause conflicts with that)
Convert just Translator.java to Kotlin, so that we have named arguments, which gives us the same reviewable safety as the builder pattern.
I'm curious what the Android team's preference is. I think 3 would be very simple, as it's a very straightforward file that Android Studio auto-conversion should handle without problems, but it would be a change from our current policy of only using Java for non-generated code.