Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Rework expression api #11210

Merged
merged 1 commit into from
Feb 26, 2018
Merged

Rework expression api #11210

merged 1 commit into from
Feb 26, 2018

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Feb 15, 2018

This PR reworks the expression API to remove the generic type (for context, see #11028).

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Feb 15, 2018
@tobrun tobrun added this to the android-v6.0.0 milestone Feb 15, 2018
@tobrun tobrun self-assigned this Feb 15, 2018
@tobrun
Copy link
Member Author

tobrun commented Feb 15, 2018

@jfirebaugh @anandthakker

I made two proposal to address the comments from #11028

The former branch (this PR) removes the generic type. For api we expose that an expression can take another expression as input + we expose some facility methods for simple expressions to remove the necessity of using literal():

eg.

get(literal("name")) is shortened to get("name")

The latter branch reworks the API to remove all types and relies on using object.java. This makes the api a little less discoverable but has the benefit of not needing to wrap literals in complex expressions.

eg.

match expression from this PR
        match(
          get("name"),
          literal("Westerpark"), rgb(255, 0, 0), 
          literal("Jordaan"), rgb(0, 255, 0),
          literal("Prinseneiland"), rgb(0, 0, 255),
          rgb(255, 255, 255)
        )
match expression from second proposal
        match(
          get("name"),
          "Westerpark", rgb(255, 0, 0),
          "Jordaan", rgb(0, 255, 0),
          "Prinseneiland", rgb(0, 0, 255),
          rgb(255, 255, 255)
        )

Thoughts/Preferences?

cc @ivovandongen @LukasPaczos

@tobrun tobrun merged commit 727afc4 into release-boba Feb 26, 2018
@tobrun tobrun deleted the tvn-rework-expression-api branch February 26, 2018 12:35
@LukasPaczos LukasPaczos mentioned this pull request Mar 2, 2018
23 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants