mapbox / mapbox-maps-android

Interactive, thoroughly customizable maps in native Android powered by vector tiles and OpenGL.
https://www.mapbox.com/mobile-maps-sdk
Other
478 stars 134 forks source link

Enum value and valueOf string value is not same for IconAnchor and TextAnchor #2503

Closed afarhan39 closed 3 weeks ago

afarhan39 commented 4 weeks ago

Environment

Observed behavior and steps to reproduce

From path: com/mapbox/maps/extension/style/layers/properties/generated/Property.kt

companion object {
    /**
     * The center of the icon is placed closest to the anchor.
     */
    @JvmField
    val CENTER = IconAnchor("center")
    /**
     * The left side of the icon is placed closest to the anchor.
     */
    @JvmField
    val LEFT = IconAnchor("left")
    /**
     * The right side of the icon is placed closest to the anchor.
     */
    @JvmField
    val RIGHT = IconAnchor("right")
    /**
     * The top of the icon is placed closest to the anchor.
     */
    @JvmField
    val TOP = IconAnchor("top")
    /**
     * The bottom of the icon is placed closest to the anchor.
     */
    @JvmField
    val BOTTOM = IconAnchor("bottom")
    /**
     * The top left corner of the icon is placed closest to the anchor.
     */
    @JvmField
    val TOP_LEFT = IconAnchor("top-left")
    /**
     * The top right corner of the icon is placed closest to the anchor.
     */
    @JvmField
    val TOP_RIGHT = IconAnchor("top-right")
    /**
     * The bottom left corner of the icon is placed closest to the anchor.
     */
    @JvmField
    val BOTTOM_LEFT = IconAnchor("bottom-left")
    /**
     * The bottom right corner of the icon is placed closest to the anchor.
     */
    @JvmField
    val BOTTOM_RIGHT = IconAnchor("bottom-right")

    /**
     * Utility function to get [IconAnchor] instance from given [value].
     */
    @JvmStatic
    fun valueOf(value: String): IconAnchor {
      return when (value) {
        "CENTER" -> CENTER
        "LEFT" -> LEFT
        "RIGHT" -> RIGHT
        "TOP" -> TOP
        "BOTTOM" -> BOTTOM
        "TOP_LEFT" -> TOP_LEFT
        "TOP_RIGHT" -> TOP_RIGHT
        "BOTTOM_LEFT" -> BOTTOM_LEFT
        "BOTTOM_RIGHT" -> BOTTOM_RIGHT
        else -> throw RuntimeException("IconAnchor.valueOf does not support [$value]")
      }
    }
  }

Similar things also can be observed in TextAnchor

both enum value and valueOf string value is not same, for example: center != CENTER

So, what happened currently, its always throwing error IconAnchor.valueOf does not support center

Expected behavior

I'm expecting both string should match, ie val CENTER = IconAnchor("CENTER") which later can correctly mapped via IconAnchor.valueOf("CENTER")

kiryldz commented 3 weeks ago

@afarhan39 what is the specific bug you're facing with that? Our internal usage of valueOf e.g. here applies needed transformations so that values can get mapped as expected. I assume you should either do the same if you for some reason need to perform this mapping manually or just use our high-level APIs like:

if (lineLayer.lineJoin == LineJoin.ROUND) {
 // do something
}

Our APIs were actually designed for users not to mess up with strings and provide high-level typed APIs.

afarhan39 commented 3 weeks ago

I had a usecase that require to store IconAnchor into data class

below is the code

@Parcelize
data class CustomLocationMarkerData(
    val locationData: LocationData,
    val iconType: Constants.IconType,
    val locationName: String? = null,
    val iconText: String? = null,
    val id: String? = null,
    val anchorPoint: String? = null,
    val sortKey: Double? = null
) : Parcelable {
    companion object {
        val anchorCenter = IconAnchor.CENTER.value
    }
}

and since the workaround is storing string value, when I remap again via IconAnchor.valueOf(anchorPoint), its throwing error IconAnchor.valueOf does not support center Doesn't enum value should be the same? I don't get why the string value is different

kiryldz commented 3 weeks ago

Consider valueOf as an internal utility function. I explained in https://github.com/mapbox/mapbox-maps-android/issues/2503#issuecomment-2437601119 how it works in our SDK. The reason why it was designed like that is because this code is generated and it was easier for us to match the way of how we generate actual enum declaration and related valueOf function. In your case you can just map it the same way we do here.

afarhan39 commented 3 weeks ago

gotcha, that will work. thanks!