Forum PHP.pl

Chciałem się zapytać czy dobrze rozwiązałem poniższy problem:

Mam takie encje:

[SQL] pobierz, plaintext 
users {id}
matchdays {id}
matches {id,matchday_id}
points {id,user_id,match_id,numberOfpoints}
[SQL] pobierz, plaintext 

Potrzebuję pobrać dane tak aby wyświetlić dla każdego użytkownika sumy punktów dla każdej osobnej kolejki plus na końcu sumę wszystkich punktów ze wszystkich kolejek , tak aby później wyświetlić to w widoku TWIG, coś w tym stylu:

Imie /k1 /k2 /k3 /k4 /k5 / suma
---------------------------------
User3 / 4 / 2 / 4 / 2 / 4 / 16
User1 / 2 / 0 / 4 / 4 / 2 / 12
User2 / 2 / 0 / 2 / 4 / 2 / 1

Zrobiłem to tak:

[PHP] pobierz, plaintext 
namespace My\TyperkaBundle\Repository;
use Doctrine\ORM\EntityRepository;
 
class TypeRepository extends EntityRepository {
 
    private $sum_per_user = array();
    private $users = array();
    private $final_result = array();
 
    public function getPointsPerMatchday(){
 
        $qb = $this->createQueryBuilder('t');
        $qb->select(
                    'SUM(t.numberOfPoints) AS suma'
                    ,'u.username AS username'
                    ,'u.id AS user'
                    ,'u.priority'
                    ,'md.id as matchday'
                )
           ->innerJoin('t.match', 'm')
           ->innerJoin('m.matchday', 'md')     
           ->innerJoin('t.user', 'u')
           ->where('md.id BETWEEN 1 AND 15')
           ->groupBy('u.username, md.id')
        ; 
 
        $result = $qb->getQuery()->getResult();
 
        // 1. Pobranie tylko użytkowników
        foreach ($result as $details){
            $this->users[] = $details['user'];
        }
 
        // 2. Wybranie unikalnych wartości
        $this->users = array_unique($this->users);
 
        // 3. Uporządkowanie kluczy
        $keys = array(0,1,2,3,4,5,6,7,8,9);
        $this->users = array_combine($keys, $this->users);
 
        // 4. Zsumowanie punktów dla każdego użytkownika
        for($i=0;$i<10;$i++){
            foreach ($result as $details){
                if($this->users[$i] == $details['user']){
                    if(!isset($this->sum_per_user[$this->users[$i]])){
                        $this->sum_per_user[$this->users[$i]] = 0;
                    }
                    $this->sum_per_user[$this->users[$i]] = $this->sum_per_user[$this->users[$i]] + (int)$details['suma'];
                }
            }
        }
 
        // 5. Posortowanie użytkowników wg sumy punktów
        arsort($this->sum_per_user);
 
        // 6. Kolejne uporządkowanie kluczy 
        $this->users = array_combine($keys, array_keys($this->sum_per_user));
 
        // 7. Uporządkowanie danych użytkowników wg sumy punktów ze wszystkich kolejek
        // oraz zmiana wyglądu tablicy na: dla każdego użytkownika sumy punktów w każdej kolejce
        foreach($this->users as $user){
            foreach ($result as $details){
                if($user == $details['user']){
                    if(isset($user)){
                        $this->final_result[$details['username']][] = (int)$details['suma'];
                    }
                }
            }
        }
 
        return $this->final_result; 
    }
[PHP] pobierz, plaintext 

Proszę o uwagi. Co zrobiłem źle ? Jak można to inaczej (może prościej) napisać ?
Będę wdzięczny za poprawki do kodu.

W jakim sensie zrobiles zle? Dane ci wyswietlaja nie tak jak chcesz czy moze poprostu pytasz czy mozna to zrobic optymalniej?

Dane wyświetlają się tak jak powinny a więc tutaj jestem zadowolony.

Pytam właśnie czy można to zrobić lepiej/porządniej/profesjonalniej w Symfony ?
Nie wiem po prostu czy nie napsiałem za dużo kodu, czy pokazując taki kod komuś nie powinienem się wstydzić (IMG:style_emoticons/default/wink.gif)

z punktami 1 2 3 faktycznie troche przekombinowales...Wszystko to da sie zrobic tak:

[PHP] pobierz, plaintext 
        foreach ($result as $details){
            $this->users[$details['user']] = $details['user'];
        }
[PHP] pobierz, plaintext 

i juz. Masz zalatwione duplikaty tez tym sposobem. Dodaj tylko sortowanie do select po userid i po sprawie.

Nie nadawaj aliasu na ID jako user.... to jest user_id a nie user. to dwie rozne rzeczy.
Czemu zakladasz ze mozesz miec max 10 userow - kolejna rzecz do poprawki. Pisze petle uniwersalne.

Poza tym punnkty 1 2 3 4 mozna zrobic w jednej petli zamiast 1000 razy latac po tej samej petli w roznych czesciach kodu. Tu masz grupowanie:
http://nospor.pl/grupowanie-wynikow.html
Nie jest to co ty tu masz, ale rozniez uzywa grupowania w jednym obiegu petli zamiast w kilku.

Dalszych punktow nie sprawdzalem, bo troche mam wlasnej roboty

trochę mnie natchnąłeś ... i udało się fajnie zoptymalizować:

Podsumujmy:

// z SQLa miałem taką tablicę:

[HTML] pobierz, plaintext 
$result = array(
    0 => array("suma" => "26","username" => "Adam 1","user" => 9,"priority" => 1,"matchday" => 1),
    1 => array("suma" => "23","username" => "Adam 1","user" => 9,"priority" => 1,"matchday" => 2),
    2 => array("suma" => "20","username" => "Adam 1","user" => 9,"priority" => 1,"matchday" => 3),
    3 => array("suma" => "18","username" => "Adam 1","user" => 9,"priority" => 1,"matchday" => 4),
    4 => array("suma" => "14","username" => "Damian","user" => 12,"priority" => 2,"matchday" => 1),
    5 => array("suma" => "22","username" => "Damian","user" => 12,"priority" => 2,"matchday" => 2),
    itd ...
);
[HTML] pobierz, plaintext 

// 1. Sumuję więc łącznie wszystkie punkty ze wszystkich kolejek dla każdego usera

[PHP] pobierz, plaintext 
    foreach($result as $details){
       if(!isset($this->users[$details['user']])){
          $this->users[$details['user']] =0;
       }
       $this->users[$details['user']] = $this->users[$details['user']] + (int)$details['suma'];
    }
[PHP] pobierz, plaintext 

// otrzymuję to:

[HTML] pobierz, plaintext 
  $users = array(
      9 => 87,
      12 => 86, 
      2 => 90,
      3 => 88,
      itd...
  );
[HTML] pobierz, plaintext 

// 2. Sortuję aby wiedzieć kto ma najwięcej punktów

[PHP] pobierz, plaintext 
arsort($this->users);
[PHP] pobierz, plaintext 

// otrzymuję to:

[HTML] pobierz, plaintext 
  $users = array(
       5 => 104,
      16 => 102,
      18 => 94,
      13 => 92,
      2 => 90,
      itd...
  );
[HTML] pobierz, plaintext 

// 3. Przypisuję punkty dla każdej kolejki osobno dla posortowanych już userów
// według łącznej sumy wszystkich punktów ze wszystkich kolejek

[PHP] pobierz, plaintext 
       foreach($this->users as $key => $value){
            foreach ($result as $details){
                if($key == $details['user']){
                    $this->final_result[$details['username']][] = (int)$details['suma'];
                }
            }
        }
[PHP] pobierz, plaintext 

// i otrzymuję to co chcę a więc to:

[PHP] pobierz, plaintext 
  $final_result = array(
      "Mateusz" => array(24,26,28,26),
      "Tomek" => array(26,24,26,26),
      "Wojtek" => array(28,22,22,22),
      itd ...
  );
[PHP] pobierz, plaintext 

można to jeszcze zoptymalizować albo zrobić lepiej ?

Ten post edytował damianooo 26.01.2016, 20:54:20

nie: $this->users[$details['user']] = $this->users[$details['user']] + (int)$details['suma'];
a: $this->users[$details['user']] += (int)$details['suma'];

prosilem bys uzywal user_id... oj, bo sie pogniewamy...


$this->final_result[$details['username']][] = (int)$details['suma'];
kluczem powinno byc id usera nie jego name. userzy teoretycznie moga miec te same name, id nie

ok poprawiłem i mam teraz tak:

[PHP] pobierz, plaintext 
        // 1. Sumuję łącznie wszystkie punkty ze wszystkich kolejek dla każdego usera
        foreach($result as $details){
            if(!isset($this->users[$details['user_id']])){
                $this->users[$details['user_id']] =0;
            }
            $this->users[$details['user_id']] += (int)$details['suma'];
        }
 
        // 2. Sortuję aby wiedzieć kto ma najwięcej punktów 
        arsort($this->users);
 
        // 3. Przypisuję punkty dla każdej kolejki osobno dla posortowanych już userów
        //    według łącznej sumy wszystkich punktów ze wszystkich kolejek
        foreach($this->users as $key => $value){
            foreach ($result as $details){
                if($key == $details['user_id']){
                    $this->final_result[$details['username']][] = (int)$details['suma'];
                }
            }
        }
[PHP] pobierz, plaintext 

Apropo tego, że kluczem powinno być id a nie nazwa usera to wiem (IMG:style_emoticons/default/smile.gif) ... ja to zrobiłem tak celowo ponieważ potrzebuję takiej właśnie tablicy aby ją wyrzucić w TWIGu ... id mi tam nie są potrzebne , to ma być wyświetlone tak:

Imie /k1 /k2 /k3 /k4 /k5 / suma
---------------------------------
User3 / 4 / 2 / 4 / 2 / 4 / 16
User1 / 2 / 0 / 4 / 4 / 2 / 12
User2 / 2 / 0 / 2 / 4 / 2 / 10

Rozumiem, że imiona mogą się powtarzać ale ja mam to w mojej aplikacji rozwiązane tak że jak jest np. dwóch Marków to jeden jest Marek 1 a drugi Marek 2

Ten post edytował damianooo 27.01.2016, 09:56:19

Nabrales zlych nawykow i zamiast je zmienic, poki masz okazje, to twardo przy nich trwasz.... Kiedys sie na tym przejedziesz, ale to juz twoja sprawa

... wolę zmienić nawyki ...

Obecnie w TWIGu miałem tak:

[HTML] pobierz, plaintext 
{% for key,point in points %}
   {{ key }}
   {% for pt in point %}
	{{ pt }}
   	{% set sum = sum + pt %}
   {% endfor %}
   {{ sum }}
{% endfor %}
[HTML] pobierz, plaintext 

jak zmienię poniższy kawałek kodu z:

[PHP] pobierz, plaintext 
$this->final_result[$details['username']][] = (int)$details['suma'];
[PHP] pobierz, plaintext 

na

[PHP] pobierz, plaintext 
$this->final_result[$details['user_id']][] = (int)$details['suma'];
[PHP] pobierz, plaintext 

to w TWIGu będę miał IDki a tego nie chcę ...

jak to powinno wyglądać ?

a więc powinno to być tak zrobione:

[PHP] pobierz, plaintext 
        foreach($this->users as $key => $value){
            foreach ($result as $details){
                if($key == $details['user_id']){
                    $this->final_result[$details['user_id']]['username'] = $details['username'];
                    $this->final_result[$details['user_id']]['suma'][] = (int)$details['suma'];
                }
            }
        }
[PHP] pobierz, plaintext 

w związku z tym TWIG powinien być taki:

[HTML] pobierz, plaintext 
    {% for key,point in points %}
       {{ point.username }}
       {% for pt in point.suma %}
    	{{ pt.suma }}
       	{% set sum = sum + pt.suma %}
       {% endfor %}
       {{ sum }}
    {% endfor %}
[HTML] pobierz, plaintext 

pięknie (IMG:style_emoticons/default/smile.gif)

dzięki nospor pozdrawiam

Czy czasem repozytoria obiektów nie powinny zwracać właśnie obiektów, które obsługują lub ich kolekcji? Może lepiej przenieść generowanie tych statystyk na zewnątrz repozytorium, np. do jakiejś logiki, np.:

[PHP] pobierz, plaintext 
namespace Costam/abc;
 
use Doctrine\Common\Collections\Collection;
 
interface CalcUserStats
{
    public function calcCosTam(Collection $users);
}
[PHP] pobierz, plaintext 

W tej sposób uniezależniasz się od źródła danych, bo równie dobrze tacy użytkownicy mogą pochodzić z cache lub jakiejś zewnętrznej usługi po REST/WS.