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

Add javadoc code examples #11540

Merged
merged 1 commit into from
Apr 10, 2018
Merged

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Mar 27, 2018

WIP, this PR adds inline expression code examples to the javadoc.

closes #11159

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Mar 27, 2018
@tobrun tobrun added this to the android-v6.0.0 milestone Mar 27, 2018
@tobrun tobrun self-assigned this Mar 27, 2018
@tobrun tobrun force-pushed the tvn-add-javadoc-code-examples branch from 87922d5 to c5a06d2 Compare March 27, 2018 20:22
@tobrun tobrun force-pushed the tvn-add-javadoc-code-examples branch from c5a06d2 to 41d89a1 Compare April 6, 2018 14:26
@tobrun
Copy link
Member Author

tobrun commented Apr 6, 2018

Ready for review @LukasPaczos

Copy link
Member

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

Great seeing code examples in the javadoc itself! 🚀

While we are at it, some nice-to-haves:

  • example usage of Expression#properties() and Expression#geometryType()
  • javadoc indication that Expression#string, Expression#number etc. return the first type-matching object rather than a boolean that it's present in the set
  • code example for exponential(@NonNull Expression expression)
  • more complicated styling example combining a couple of expressions at the top of the class?

* symbolLayer.setProperties(
* iconSize(
* switchCase(
* get(KEY_PROPERTY_SELECTED), literal(3.0f),
Copy link
Member

Choose a reason for hiding this comment

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

Maybe one more case option before default?

Copy link
Member Author

Choose a reason for hiding this comment

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

that would make it a bit more clear + also added // default value:

   * symbolLayer.setProperties(
   *     iconSize(
   *         switchCase(
   *             get(KEY_TO_BOOLEAN), literal(3.0f),
   *             get(KEY_TO_OTHER_BOOLEAN), literal(5.0f)
   *             literal(1.0f) // default value
   *         )
   *     )
   * );

Copy link
Member

Choose a reason for hiding this comment

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

Looks great 🚀

@@ -569,6 +986,7 @@ public static Expression match(@NonNull Expression input, @NonNull Expression de

/**
* Evaluates each expression in turn until the first non-null value is obtained, and returns that value.
* // TODO: 26/03/2018 javadoc example
Copy link
Member

Choose a reason for hiding this comment

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

TODO sneaked in

* {@code
* SymbolLayer symbolLayer = new SymbolLayer("layer-id", "source-id");
* symbolLayer.setProperties(
* textField(get(literal("key-to-feature"), properties()))
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this result in a call without a second argument, get("key-to-feature")? Can we add an example that uses a different properties object?

* {@code
* SymbolLayer symbolLayer = new SymbolLayer("layer-id", "source-id");
* symbolLayer.setProperties(
* textField(get("key-to-feature", properties()))
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

* {@code
* FillLayer fillLayer = new FillLayer("layer-id", "source-id");
* fillLayer.setFilter(
* has(string("keyToValue"))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be has(literal("keyToValue"))?

Copy link
Member Author

Choose a reason for hiding this comment

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

actually has(get("keyToValue"))

* FillLayer fillLayer = new fillLayer("layer-id", "source-id");
* fillLayer.setProperties(
* fillColor(
* exponential(0.5f), zoom(),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

* FillLayer fillLayer = new fillLayer("layer-id", "source-id");
* fillLayer.setProperties(
* fillColor(
* linear(), zoom(),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

* FillLayer fillLayer = new fillLayer("layer-id", "source-id");
* fillLayer.setProperties(
* fillColor(
* exponential(0.5f), zoom(),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

* FillLayer fillLayer = new fillLayer("layer-id", "source-id");
* fillLayer.setProperties(
* fillColor(
* cubicBezier(0.42f, 0.0f, 1.0f, 1.0f), zoom(),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

* FillLayer fillLayer = new fillLayer("layer-id", "source-id");
* fillLayer.setProperties(
* fillColor(
* cubicBezier(0.42f, 0.0f, 1.0f, 1.0f), zoom(),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@tobrun tobrun force-pushed the tvn-add-javadoc-code-examples branch from 41d89a1 to 109439e Compare April 10, 2018 11:04
@tobrun tobrun force-pushed the tvn-add-javadoc-code-examples branch from 109439e to 7fd0d14 Compare April 10, 2018 11:11
Copy link
Member

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

🚢

@tobrun tobrun merged commit ce75c3b into release-boba Apr 10, 2018
@tobrun tobrun deleted the tvn-add-javadoc-code-examples branch April 10, 2018 11:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants