From 3f8311d708238200a5c2cfc8daea545df15b8149 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20Vigh?= <balazs@vigh.eu>
Date: Sat, 8 May 2021 23:37:36 +0200
Subject: [PATCH] implemented review findings

---
 src/Database/Query/Select.php      |  9 ++++----
 src/Repository/PlaceRepository.php | 33 +++++++++++++++++-------------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/Database/Query/Select.php b/src/Database/Query/Select.php
index 8dc3a90..d62bd84 100644
--- a/src/Database/Query/Select.php
+++ b/src/Database/Query/Select.php
@@ -328,14 +328,13 @@ class Select
 
     private function generateJoins(): array
     {
-        $joins = $this->joins;
-        
+
         $joinQueries = [];
         $params = [];
 
-        foreach($joins as $value) {
-            list($joinQueryFragment, $paramsFragment) = $this->generateTable($value[1], true);
-            array_push($joinQueries, $value[0] . ' JOIN ' . $joinQueryFragment . ' ON ' . $this->generateColumn($value[2]) . ' ' . $value[3] . ' ' . $this->generateColumn($value[4]));
+        foreach($this->joins as $join) {
+            list($joinQueryFragment, $paramsFragment) = $this->generateTable($join[1], true);
+            $joinQueries[] = $join[0] . ' JOIN ' . $joinQueryFragment . ' ON ' . $this->generateColumn($join[2]) . ' ' . $join[3] . ' ' . $this->generateColumn($join[4]);
             $params = array_merge($params, $paramsFragment);
         }
 
diff --git a/src/Repository/PlaceRepository.php b/src/Repository/PlaceRepository.php
index aa6fac8..4c36edc 100644
--- a/src/Repository/PlaceRepository.php
+++ b/src/Repository/PlaceRepository.php
@@ -31,25 +31,30 @@ class PlaceRepository
     //TODO: use Map and User instead of id
     public function getRandomNPlaces(int $mapId, int $n, ?int $userId): array
     {
-        if(!isset($userId)) { // anonymous single player
+        if (!isset($userId)) { // anonymous single player
             return $this->getRandomNForMapWithValidPano($mapId, $n);
         } else { // authorized user or multiplayer game with selection based on what the host played before
             $unvisitedPlaces = $this->getRandomUnvisitedNForMapWithValidPano($mapId, $n, $userId);
+            if (count($unvisitedPlaces) == $n) {
+                return $unvisitedPlaces;
+            }
+
             $oldPlaces = $this->getRandomOldNForMapWithValidPano($mapId, $n - count($unvisitedPlaces), $userId);
+            
             return array_merge($unvisitedPlaces, $oldPlaces);
         }
     }
 
     //TODO: use Map instead of id
-    public function getRandomNForMapWithValidPano(int $mapId, int $n, array $exclude = []): array
+    private function getRandomNForMapWithValidPano(int $mapId, int $n): array
     {
         $places = [];
 
         $select = new Select(\Container::$dbConnection, 'places');
-        $select->where('id', 'NOT IN', $exclude);
         $select->where('map_id', '=', $mapId);
         $numberOfPlaces = $select->count();
 
+        $exclude = [];
         for ($i = 1; $i <= $n; ++$i) {
             $place = $this->getRandomForMapWithValidPano($numberOfPlaces, $select, $exclude);
 
@@ -60,31 +65,31 @@ class PlaceRepository
         return $places;
     }
 
-    private function getRandomForMapWithValidPano($numberOfPlaces, $select, array &$exclude, ?callable $pickRandomInt = null): ?Place
+    private function getRandomForMapWithValidPano(int $numberOfPlaces, Select $select, array &$exclude, ?callable $pickRandomInt = null): ?Place
     {
         do {
             $numberOfPlacesLeft = $numberOfPlaces - count($exclude);
             $place = $this->selectRandomFromDbForMap($numberOfPlacesLeft, $select, $exclude, $pickRandomInt);
-            if($place === null) {
+            if ($place === null) {
                 // there is no more never visited place left
                 return null;
             }
 
             $panoId = $place->getFreshPanoId();
-            if($panoId === null) {
+            if ($panoId === null) {
                 $exclude[] = $place->getId();
             }
-        } while($panoId === null);
+        } while ($panoId === null);
 
         return $place;
     }
 
-    private function selectRandomFromDbForMap($numberOfPlacesLeft, $select, array $exclude, ?callable $pickRandomInt): ?Place
+    private function selectRandomFromDbForMap(int $numberOfPlacesLeft, Select $select, array $exclude, ?callable $pickRandomInt): ?Place
     {
-        if($numberOfPlacesLeft <= 0)
+        if ($numberOfPlacesLeft <= 0)
             return null;
 
-        if(!isset($pickRandomInt)) {
+        if (!isset($pickRandomInt)) {
             $randomOffset = random_int(0, $numberOfPlacesLeft - 1);
         } else {
             $randomOffset = $pickRandomInt($numberOfPlacesLeft);
@@ -118,11 +123,11 @@ class PlaceRepository
         // look for as many new places as possible but maximum $n
         do {
             $place = $this->getRandomForMapWithValidPano($numberOfUnvisitedPlaces, $selectUnvisited, $exclude);
-            if(isset($place)) {
+            if (isset($place)) {
                 $places[] = $place;
                 $exclude[] = $place->getId();
             }
-        } while(count($places) < $n && isset($place));
+        } while (count($places) < $n && isset($place));
 
         return $places;
     }
@@ -159,10 +164,10 @@ class PlaceRepository
         };
 
         // look for n - numberOfUnvisitedPlaces places
-        while(count($places) < $n)
+        while (count($places) < $n)
         {
             $place = $this->getRandomForMapWithValidPano($numberOfOldPlaces, $selectOldPlaces, $exclude, $pickGaussianRandomInt);
-            if(isset($place))
+            if (isset($place))
             {
                 $places[] = $place;
                 $exclude[] = $place->getId();