amplitude / Amplitude-Kotlin

Amplitude Kotlin SDK
MIT License
27 stars 10 forks source link

Reused enum properties are duplicated for each event in Ampli wrapper #192

Open JonatanPlesko opened 3 months ago

JonatanPlesko commented 3 months ago

I'm unsure whether this should be reported on the Android SDK, but I figured I could report it here since it is about generated Android code and I cannot find a repository for the Ampli CLI.

In case you have one enum property in your tracking plan, which is assigned to multiple events, the Ampli.kt wrapper will generate a separate instance for each event. For example, one might have a deviceType property which they use for two events - Connect to Device and Connect to Device Failed. Ampli.kt will contain ConnectToDevice and ConnectToDeviceFailed classes, but they will both have a DeviceType enum within them (the identical one) - ConnectToDevice.DeviceType and ConnectToDeviceFailed.DeviceType.

This makes it impossible to reuse mapping between the app's model and Ampli's model, as each event needs to have its own mapping for the exact same property.

Expected Behavior

Ampi.kt should only contain one class for each enum property instead of nesting them within events.

Current Behavior

Ampi.kt has nested enum classes for each event that uses an enum property.

Possible Solution

In general, I do not see the point in nesting properties within event classes as they're also completely separate in Amplitude Data. Code generation could just avoid nesting altogether, or only nest when it is necessary (some kind of conflict resolution).

Steps to Reproduce

  1. Create an enum property
  2. Add the enum property to multiple events
  3. Do ampli pull and look at the generated code.

Environment