search

sharewire / google-maps-clustering

Fast marker clustering library for Google Maps Android API.
Apache License 2.0
292 stars 77 forks source link

Make ClusterRenderer exchangable #10

Closed meierjan closed 6 years ago

meierjan commented 6 years ago

In my sideproject I wanted to exchange the renderer to change how tag's are set and the animation. So I thought sharing this is a good idea.

Another way of archiving this would be by defaulting to the ClusterRenderer and having a setter that makes it possible to exchange it later on.

You are more then welcome to give some thoughts on that.

makovkastar commented 6 years ago

To be honest I'm not a fan of having a base class and then extending it with more functionality. Effective Java Item 17 says "Design and document for inheritance, or else prohibit it". In my case ClusterRenderer was not designed for inheritance, which can cause issues. One way to go is to have an interface ClusterRenderer and then a default implementation DefaultClusterRenderer.

meierjan commented 6 years ago

Good objection. I will change everything accordingly.

makovkastar commented 6 years ago

I'm going to close this PR. Please open a new one if necessary.