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

Make layer and source ownership consistent #6995

Closed
4 tasks done
ivovandongen opened this issue Nov 10, 2016 · 4 comments
Closed
4 tasks done

Make layer and source ownership consistent #6995

ivovandongen opened this issue Nov 10, 2016 · 4 comments
Assignees
Labels
Android Mapbox Maps SDK for Android

Comments

@ivovandongen
Copy link
Contributor

ivovandongen commented Nov 10, 2016

Like iOS in #6254, we need some refactoring to make the layer/source ownership more convenient for developers. Atm, after adding a newly created layer/source to the map, the original java reference becomes unusable. To facilitate this a bit, we throw a Java exception to report misuse of the api to the developer. But this is not well understood.

Instead, we can also keep a raw pointer to the layer/source after transferring ownership and make sure the reference to the map is set.

ToDo:

  • Get rid of explicit checks for validity
  • Refactor core object transfer so that the native map reference is set when adding the layer/source to the map (needed for CustomLayer::update)
  • Ensure that adding a layer/source to the map that was already added throws a nice Java exception
  • Add JavaDoc:
    • MapboxMap#addLayer
    • MapboxMap#removeLayer
    • MapboxMap#addSource
    • MapboxMap#removeSource
@ivovandongen ivovandongen added the Android Mapbox Maps SDK for Android label Nov 10, 2016
@ivovandongen ivovandongen added this to the android-v4.2.0 milestone Nov 10, 2016
@ivovandongen ivovandongen self-assigned this Nov 10, 2016
@jfirebaugh
Copy link
Contributor

#6959 and the discussion there is also relevant.

@boundsj
Copy link
Contributor

boundsj commented Nov 11, 2016

I proposed #7014 for #6959. If there is interest in using in for Android see #7014 (comment)

@ivovandongen
Copy link
Contributor Author

@jfirebaugh @boundsj Thanks, I missed that ticket completely.

@boundsj If you could get your PR merged, I'll make the necessary changes to this one to take advantage of it for Android.

@ivovandongen
Copy link
Contributor Author

@jfirebaugh @boundsj I've created a separate ticket (#7023) for the ownership transfer on remove so these changed are not held up by that.

cc @tobrun @zugaldia

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

No branches or pull requests

3 participants