Update setMinClusterLocations()#3
Conversation
Added simple sanity check because doesn't make sense to have clusters with less than 2 locations. It also has to be an integer and positive number.
| { | ||
| $this->minLocations = $limit; | ||
| // simple sanity check. It doesn't make sense to have clusters with less than 2 locations | ||
| $this->minLocations = (int)($limit > 2 ? $limit : 0); |
There was a problem hiding this comment.
Instead of automatically changing, can you make it throw an exception instead? (and add the @throws to this PHPDoc)
if ($limit < 2) {
throw new Exception("It doesn't make sense to have clusters with less than 2 locations.");
}
$this->minLocations = $limit;And can you also fix up ClustererTest.php, which relies on clusters of 1 being possible?
testClusterCoordinatesException calls setMinClusterLocations with 1, which can safely be changed to 2.
testClustered also calls it with 1. You can also change it to 2, but part of the test will have to be changed then:
// all coordinates should be clustered
// the 3 Brussels railway stations should form 1 cluster
$clusters = $clusterer->getClusters();
$this->assertEquals(count($coordinates) - 2, count($clusters));
$this->assertEquals(3, $clusters[3]->total);
$this->assertEquals(0, count($clusterer->getCoordinates()));
// cluster must also have empty coordinate array
$this->assertCount(0, $clusters[0]->coordinates);There was a problem hiding this comment.
Currently I working on a script to automatically set the number of minLocations based on the zoom level and on the number of markers on the map. Sometimes this script provides a small number between 0 and 2. This sanity check is being done in this script, however I believe it should be done in the library in order to avoid problems. Throwing an exception in this case (and in my opinion) is not advised, due to that it is more appropriate to just change it. Also adding an exception would cause an incompatibility with older versions, requiring developers to adjust their code.
I would only throw an exception if debug mode was on....
5298af7 to
bcaac43
Compare
Added simple sanity check because doesn't make sense to have clusters with less than 2 locations.
It also has to be an integer and positive number.