search

TradeMe / MapMe

The Android maps adapter
MIT License
841 stars 75 forks source link

Issue #9: Improve Factory method #10

Open marcelpinto opened 7 years ago

marcelpinto commented 7 years ago

Only accept the position of the marker so the onCreateAnnotation is clearer and does not duplicate the work that should be done in onBindAnnotation. That simplifies the adapter and avoid possible duplication of work and resources.

josh-burton commented 7 years ago

Hi, thanks for the pull request.

I completely agree with you - it is a little confusing. I'll have a bit more of a think around onCreate vs onBind.

One thing I will say in the meantime is that we'll need to maintain backwards compatibility, so rather than removing methods from the annotation factory, can you deprecate them instead?

marcelpinto commented 7 years ago

Ok I thought that you were still under beta so no need of backward compatibility. I will try to refactor it.

Actually, I have some doubts on the need of the AnnotationFactory to create markers. Why not let the user create the markers without need of the factory.

Another point is that MapAnnotation exposes addToMap, remove and annotate, this might lead to misusage of it.

josh-burton commented 7 years ago

The factory is there as an abstraction on top of the underlying map. By using the factory, it makes it really easy to swap out the map provider by just changing the parent adapter class.

Good point about those public methods though!