Improved CustomGeometrySource constructor typing #13200
Conversation
LukasPaczos
added
Android
| * @param options CustomGeometrySourceOptions. | ||
| * @param provider The tile provider that returns geometry data for this source. | ||
| */ | ||
| @UiThread | ||
| public CustomGeometrySource(String id, CustomGeometrySourceOptions options, | ||
| public CustomGeometrySource(String id, GeoJsonOptions options, |
There was a problem hiding this comment.
I'm not sure I understand the typing problem being addressed here. The documentation is missing the wrap and clip options that are also supported. If those are supported, would it be better to keep the CustomGeometrySourceOptions type so that a developer knows the right type to instantiate to access those additional properties?
There was a problem hiding this comment.
The constructor accepts also GeoJsonOptions base type and to make this abstraction easier to implement, without casting, I think that we should accept the base type as the argument type. The CustomGeometrySourceOptions, which contains those additional options, is referenced in the javadoc as well.
Would adding another constructor for the extended type make it more readable?
There was a problem hiding this comment.
Would adding another constructor for the extended type make it more readable?
That wouldn't really help, as it would only be used automatically in a few cases. One option is to remove the inheritance in CustomGeometrySourceOptions and have it only contain the supported methods, returning the correct object type.
There was a problem hiding this comment.
Sure @asheemmamoowala, this makes sense.
tobrun
force-pushed
the
android-semver
branch
from
c567774
to
03671ea
Compare
|
Can you retarget this branch to master? |
tobrun
force-pushed
the
android-semver
branch
from
fdd5f59
to
020966a
Compare
LukasPaczos
force-pushed
the
lp-improve-cgs-constructor-typing
branch
from
1948a58
to
432b097
Compare
LukasPaczos
force-pushed
the
lp-improve-cgs-constructor-typing
branch
from
432b097
to
07fdd55
Compare
|
Read for another round. |
Closes #12009.