Replace deprecated maker usage in NavigationMapboxMap with SymbolManager #1707
Conversation
danesfeder
added
✓ ready for review
backwards incompatible
danesfeder
force-pushed
the
dan-annotation-plugin
branch
from
fbeb65a to
7be8915
Compare
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| class MapSymbolManager { |
There was a problem hiding this comment.
What about renaming this to be NavigationSymbolManager, to make it more clear that this is a Navigation wrapper for SymbolManager?
| private final List<Symbol> mapMarkersSymbols = new ArrayList<>(); | ||
| private final SymbolManager symbolManager; | ||
|
|
||
| MapSymbolManager(SymbolManager symbolManager) { |
There was a problem hiding this comment.
How about adding an overloaded constructor that takes the parameters needed to instantiate the SymbolManager to remove that extra step? i.e., MapSymbolManager(MapView mapView, MapboxMap mapboxMap) { this(new SymbolManager(mapView, mapboxMap, mapboxMap.getStyle())); }
There was a problem hiding this comment.
@devotaaabel are you speaking in terms of the work to do this in NavigationMapboxMap? The reason the SymbolManager is "injected" is for easier unit testing.
...n-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/map/NavigationMapboxMap.java
Show resolved
Hide resolved
danesfeder
force-pushed
the
dan-annotation-plugin
branch
from
7be8915 to
7ccbddd
Compare
Codecov Report
@@ Coverage Diff @@
## master #1707 +/- ##
===========================================
+ Coverage 29.16% 29.36% +0.2%
- Complexity 810 820 +10
===========================================
Files 202 203 +1
Lines 7913 7913
Branches 619 619
===========================================
+ Hits 2308 2324 +16
+ Misses 5384 5368 -16
Partials 221 221 |
danesfeder
force-pushed
the
dan-annotation-plugin
branch
from
7ccbddd to
8e1a0b4
Compare
|
@devotaaabel this is ready for another round of review. I don't think it's necessary to push logic into a new constructor, then have another constructor for testing - if we can accomplish this in a cleaner manner, let's please discuss this further. |
4 participants
Closes #1706