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

Raw expression support #12007

Merged
merged 1 commit into from
May 28, 2018
Merged

Raw expression support #12007

merged 1 commit into from
May 28, 2018

Conversation

LukasPaczos
Copy link
Contributor

Closes #11158.

@LukasPaczos LukasPaczos added the Android Mapbox Maps SDK for Android label May 25, 2018
@LukasPaczos LukasPaczos added this to the android-v6.2.0 milestone May 25, 2018
@LukasPaczos LukasPaczos requested a review from tobrun May 25, 2018 11:46
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good couple of remarks before 🚢

}

private static Gson getGsonInstance() {
return gson = gson != null ? gson : new Gson();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the need for lazy initialising? Think we can directly initialise it in the field without too much overhead.

String string = (String) object;
if (string.length() > 1 &&
string.charAt(0) == '\"' && string.charAt(string.length() - 1) == '\"') {
object = string.substring(1, string.length() - 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

above code in separate method?

@@ -3452,6 +3456,9 @@ private Object toValue(ExpressionLiteral expressionValue) {

/**
* Returns a string representation of the object that matches the definition set in the style specification.
* <p>
* If this expression contains a coma (,) delimited literal, like 'rgba(r, g, b, a)`,
* it will be enclosed with double quotes (").
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to close </p> javadoc paragraph

stop("layer1", 2),
stop("layer2", 2.7));
assertEquals("expressions should match", expected, raw(expected.toString()));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract tests into separate tests methods to explain intent / track down test faillure

@LukasPaczos LukasPaczos force-pushed the 11158-raw-expression-support branch from 733b89c to c9d188c Compare May 28, 2018 12:05
if (object instanceof String) {
object = unwrapStringLiteral((String) object);
} else if (object instanceof Number) {
object = ((Number) object).floatValue();
Copy link
Contributor Author

@LukasPaczos LukasPaczos May 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting, that in order to make serialization/deserialization and testing of raw expressions easier I'm converting every Number literal to a Float to keep it consistent since JSON format isn't integer-aware. This forced adjustments in a lot of test cases.

@LukasPaczos
Copy link
Contributor Author

Ready for another round @tobrun.

@LukasPaczos LukasPaczos merged commit 22741a0 into master May 28, 2018
@LukasPaczos LukasPaczos deleted the 11158-raw-expression-support branch May 28, 2018 12:58
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.

2 participants