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

[ios, macos] Add lookup and feature operators. #11528

Merged
merged 6 commits into from
Mar 28, 2018

Conversation

fabian-guerra
Copy link
Contributor

@fabian-guerra fabian-guerra commented Mar 26, 2018

Fixes #11006

Add operators

  • geometry-type.
  • id.
  • properties.
  • at.
  • has.
  • Tests.

@fabian-guerra fabian-guerra added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS beta blocker Blocks the next beta release labels Mar 26, 2018
@fabian-guerra fabian-guerra added this to the ios-v4.0.0 milestone Mar 26, 2018
@fabian-guerra fabian-guerra self-assigned this Mar 26, 2018
@fabian-guerra fabian-guerra requested a review from 1ec5 March 26, 2018 20:41
@fabian-guerra fabian-guerra changed the title [ios, macos] Add Expressions 'at' operator. [ios, macos] Add lookup and feature operators. Mar 26, 2018
@1ec5
Copy link
Contributor

1ec5 commented Mar 27, 2018

By the way, I’ve been using this header as a guide to which built-in syntaxes (like subscripting) correspond to which built-in expression functions (objectFrom:withIndex:).

@@ -639,6 +663,9 @@ - (id)mgl_jsonExpressionObject {
} else if ([function isEqualToString:@"stringByAppendingString:"]) {
NSArray *arguments = self.arguments.mgl_jsonExpressionObject;
return [@[@"concat", self.operand.mgl_jsonExpressionObject] arrayByAddingObjectsFromArray:arguments];
} else if ([function isEqualToString:@"objectAtIndex:"]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[NSExpression expressionWithFormat:@"{1, 2, 3}[index]"] has a function of objectFrom:withIndex: and an arguments consisting of the expression index.

@@ -456,6 +465,12 @@ + (instancetype)mgl_expressionWithJSONObject:(id)object {
return [NSExpression expressionForVariable:@"zoomLevel"];
} else if ([op isEqualToString:@"heatmap-density"]) {
return [NSExpression expressionForVariable:@"heatmapDensity"];
} else if ([op isEqualToString:@"geometry-type"]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Conventional custom functions should be prefixed with mgl_ to avoid conflicting with functions that Apple may add to NSExpression in the future.

@@ -544,6 +559,15 @@ - (id)mgl_jsonExpressionObject {
if ([self.variable isEqualToString:@"zoomLevel"]) {
return @[@"zoom"];
}
if ([self.variable isEqualToString:@"geometryType"]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This SDK is using the $variable syntax both for these special built-in values that vary based on the execution context and also for user-defined variables. The assumption with zoomLevel and the like is that developers will be less likely to need a variable by that exact name.

if ([self.variable isEqualToString:@"geometryType"]) {
return @[@"geometry-type"];
}
if ([self.variable isEqualToString:@"featureId"]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency with Cocoa and this SDK, either spell this function featureID or, better yet, featureIdentifier.

XCTAssertEqualObjects([NSExpression mgl_expressionWithJSONObject:jsonExpression], expression);
}
{
NSExpression *expression = [NSExpression expressionWithFormat:@"FUNCTION('x', 'has')"];
Copy link
Contributor

Choose a reason for hiding this comment

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

The has operator has two forms. You can think of the ["has", "something"] form as shorthand for ["has", "something", ["properties"]]. This PR only implements one of the two forms.

["has", "something"] should translate to FUNCTION(self, 'mgl_has:', 'something'), and ["has", "something", object] should translate to FUNCTION(object, 'mgl_has:', 'something').

NSArray *subexpressions = MGLSubexpressionsWithJSONObjects(argumentObjects);
NSExpression *operand = argumentObjects.count > 1 ? subexpressions[1] : [NSExpression expressionWithFormat:@"self"];
NSExpression *property = subexpressions.firstObject;
return [NSExpression expressionWithFormat:@"FUNCTION(%@, 'mgl_has:', %@)", operand, property];
Copy link
Contributor

Choose a reason for hiding this comment

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

By the way, a preexisting way to write this in an expression would be FUNCTION(%@, 'valueForKey:', %@) != nil. That seems a bit more natural, although I guess mgl_has: will be slightly more ergonomic once we implement an aftermarket function for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this case mgl_has: seems like the most ergonomic option as its closer to our spec.

if ([self.variable isEqualToString:@"mgl_geometryType"]) {
return @[@"geometry-type"];
}
if ([self.variable isEqualToString:@"featureIdentifier"]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

mgl_geometryType is prefixed with the class prefix, but featureIdentifier and featureProperties are not.

@@ -651,6 +690,12 @@ - (id)mgl_jsonExpressionObject {
return @[@"to-string", self.operand.mgl_jsonExpressionObject];
} else if ([function isEqualToString:@"noindex:"]) {
return self.arguments.firstObject.mgl_jsonExpressionObject;
} else if ([function isEqualToString:@"mgl_has:"]) {
NSMutableArray *expressionObject = [NSMutableArray arrayWithObjects:@"has", self.arguments[0].mgl_jsonExpressionObject, nil];
if (self.operand.expressionType == NSConstantValueExpressionType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

FUNCTION($featureProperties, 'mgl_has:', 'property') should be translated as ["has", ["properties"], "property"], so we need to check here that the operand isn’t an NSEvaluatedObjectExpressionType (i.e., SELF).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
beta blocker Blocks the next beta release iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants