Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Wyszukiwarka ofert pracy - prośba o opinie
Forum PHP.pl > Forum > PHP
Panicz74
Witam,

Jestem trochę początkujący w PHP, ale trochę czytam, nawet troszkę w Symfony zacząłem się bawić. Chciałbym pokazać wam mój skrypt, który służy do wyświetlania i filtracji ofert pracy z bazy danych. Do wyboru są pola: nazwa branży, rodzaj umowy, lokalizacja. Chciałbym poznać waszą opinię na jego temat, co jest źle || czego za dużo || czego za mało || jakie rozwiązania są lepsze od zastosowanego, itd. Wszystkie hejty mile widziane smile.gif

Oto kod: http://pastebin.com/G9eu90d2
markuz
Na początek postaraj się oddzielic logikę od wyświetlania, na górze pliku sprawdź sobie $_GET, pobierz dane itp. a na dole korzystaj już z gotowych danych i tylko wyświetlaj.
Nie łącz się z bazą danych kilka razy - wystarczy raz.

Symfony jest trochę ciężkie dla początkującego, może zacznij od CodeIgniter bo chyba jest najłatwiejszy.
lukaskolista
Nie zaczynaj od żadnych code igniterów ani innych przestarzałych narzędzi, bo za 2 lata będziesz żałował straconego czasu. Symfony jest trudne? Od kiedy? Przecież na początku nie będzie pisał publicznych bundli i nie musi nic wiedzieć o "bebechach" symfony jak extensiony i compile passy. Moim zdaniem im szybciej zaczniesz poznawać profesjonalne rozwiązania tym lepiej. Zend też jest ok (oczywiście wersja 2) ale jak dla mnie zbyt sformalizowany. Symfony wbrew pozorom jest lekkie, tylko żeby dojść do takiego wniosku trzeba je dobrze poznać (ta potęga DI i service containera).
Panicz74
Właśnie próbuję odchudzić ten kod. Chciałem zrobić to na switchu ale mając 3x3 możliwości z GET chyba się to mija z celem. Takie coś nie działa:

  1. else if(!empty($_GET['nazwa']))
  2. {
  3. switch($_GET['nazwa'] && $_GET['umowa'] && $_GET['lokalizacja'])
  4. {
  5. //2
  6. case $_GET['nazwa'] !== "---" && $_GET['umowa'] == "---" && $_GET['lokalizacja'] == "---":
  7. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  8. LEFT JOIN users u ON u.id_usera = o.id_usera
  9. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  10. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  11. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  12. WHERE b.nazwa_branzy = :nazwa_branzy";
  13. break;
  14. //3
  15. case $_GET['nazwa'] !== "---" && $_GET['umowa'] !== "---" && $_GET['lokalizacja'] == "---":
  16. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  17. LEFT JOIN users u ON u.id_usera = o.id_usera
  18. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  19. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  20. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  21. WHERE b.nazwa_branzy = :nazwa_branzy AND i.rodzaj_umowy = :umowa";
  22. break;
  23. //4
  24. case $_GET['nazwa'] !== "---" && $_GET['umowa'] !== "---" && $_GET['lokalizacja'] !== "---":
  25. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  26. LEFT JOIN users u ON u.id_usera = o.id_usera
  27. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  28. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  29. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  30. WHERE b.nazwa_branzy = :nazwa_branzy AND i.rodzaj_umowy = :umowa AND l.lokalizacja = :lokalizacja";
  31. break;
  32. //5
  33. case $_GET['nazwa'] == "---" && $_GET['umowa'] !== "---" && $_GET['lokalizacja'] == "---":
  34. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  35. LEFT JOIN users u ON u.id_usera = o.id_usera
  36. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  37. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  38. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  39. WHERE i.rodzaj_umowy = :rodzaj_umowy";
  40. break;
  41. //6
  42. case $_GET['nazwa'] == "---" && $_GET['umowa'] == "---" && $_GET['lokalizacja'] !== "---":
  43. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  44. LEFT JOIN users u ON u.id_usera = o.id_usera
  45. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  46. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  47. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  48. WHERE l.lokalizacja = :lokalizacja";
  49. break;
  50. //7
  51. case $_GET['nazwa'] == "---" && $_GET['umowa'] !== "---" && $_GET['lokalizacja'] !== "---":
  52. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  53. LEFT JOIN users u ON u.id_usera = o.id_usera
  54. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  55. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  56. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  57. WHERE l.lokalizacja = :lokalizacja AND i.rodzaj_umowy = :rodzaj_umowy";
  58. break;
  59. //8
  60. case $_GET['nazwa'] !== "---" && $_GET['umowa'] == "---" && $_GET['lokalizacja'] !== "---":
  61. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  62. LEFT JOIN users u ON u.id_usera = o.id_usera
  63. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  64. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  65. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  66. WHERE l.lokalizacja = :lokalizacja AND b.nazwa_branzy = :nazwa_branzy";
  67. break;
  68. }
  69.  
  70. try
  71. {
  72. $pdo = new PDO($pol, $user, $pass);
  73. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  74. if(isset($_GET['nazwa']))
  75. {
  76. $stmt = $pdo->prepare($sql);
  77. if($_GET['lokalizacja'] !== "---"){$stmt -> bindValue(':lokalizacja', $_GET['lokalizacja'], PDO::PARAM_STR);}
  78. else if($_GET['nazwa'] !== "---"){$stmt -> bindValue(':nazwa_branzy', $_GET['nazwa'], PDO::PARAM_STR);}
  79. elseif($_GET['umowa'] !== "---"){$stmt -> bindValue(':umowa', $_GET['umowa'], PDO::PARAM_STR);}
  80. $stmt -> execute();
  81. echo '<ul>';
  82. foreach($stmt as $row)
  83. {
  84. echo '<li>'.$row['nazwa_branzy'].': '.$row['rodzaj_umowy'].': '.$row['login'].': '.$row['tresc'].': '.$row['lokalizacja'].': '.$row['date'].'</li>';
  85. }
  86. $stmt->closeCursor();
  87. echo '</ul>';
  88. //print_r($_GET);
  89. }
  90. }
  91. catch(PDOException $e)
  92. {
  93. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  94. }
  95. }


A co myślicie o kursach Symfony2 z Eduweb? Ktoś miał może z nimi styczność?
Pyton_000
Trochę to bez sensu..

1. Zmień name w input dla tych 3 elementów na :

where[b.nazwa_branzy], where[l.lokalizacja] i where[i.rodzaj_umowy]

Potem puste wartości zmień na puste a nie jakieś ------

w kodzie
Kod
foreach($_POST['where'] as $key => $val) {
var_dump($key, $val);
}


i dalej to już tylko doklejaj warunki.
lukaskolista
Tak, jak napisał Pyton, wartości --- zmień na puste stringi.

Tak na szybko, nie testowałem, pewnie są błędy:
  1. $mapping = array(
  2. 'nazwa' => 'nazwa_branzy',
  3. 'lokalizacja' => 'lokalizacja',
  4. 'umowa' => 'umowa',
  5. );
  6.  
  7. $sql = "
  8. SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  9. LEFT JOIN users u ON u.id_usera = o.id_usera
  10. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  11. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  12. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok
  13. ";
  14.  
  15. $where = $params = array();
  16. foreach ($_GET as $key => $value) {
  17. if (isset($mapping[$key]) && '' !== $value) {
  18. $where[] = $mapping[$key].' = :'.$mapping[$key];
  19. $params[':'.$mapping[$key]] = $value;
  20. }
  21. }
  22.  
  23. if (!empty($where)) {
  24. $sql .= " WHERE ".implode(' AND ', $where);
  25. }
  26.  
  27. $stmt = $pdo->prepare($sql);
  28. $stmt->execute($params);
Panicz74
Kurczę, słabo na razie to pojmuję, ale widzę tutaj możliwość zrobienia fajnego, dynamicznego zapytania smile.gif Już próbuje to ogarnąć smile.gif
lukaskolista
Dynamiczne zapytania realizuje się za pomocą wzorca budowniczy. Dobry przykład to Doctrine DBAL. Wbrew pozorom takie rzeźbienie czystego SQL na podstawie tablic może być trudniejsze, niż użycie jakiejś biblioteki, przynajmniej wygląda fatalnie.

Edit:
Tak konkretnie to chodziło mi o Query Builder z DBALa.
Panicz74
Zrobiłem coś takiego, na razie na jednej zmiennej, żeby sobie nie namieszać. O dziwo działa smile.gif Ale powiedzcie mi jeszcze... Jak to teraz zbindowaćquestionmark.gif Bo aż się prosi o SQL Injection?

Kod:
  1. else if(!empty($_GET['nazwa']))
  2. {
  3. $wh = array();
  4. if($_GET['nazwa'] !== "---" && $_GET['umowa'] == "---" && $_GET['lokalizacja'] == "---")
  5. {
  6. $wh[] = "b.nazwa_branzy = '{$_GET['nazwa']}'";
  7. if(!empty($wh))
  8. {
  9. $where = 'WHERE '.implode(' AND ', $wh);
  10.  
  11. }
  12. try
  13. {
  14. $pdo = new PDO($pol, $user, $pass);
  15. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  16. if(isset($_GET['nazwa']))
  17. {
  18. $sql = "SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  19. LEFT JOIN users u ON u.id_usera = o.id_usera
  20. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  21. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  22. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok ".$where;
  23. $stmt = $pdo->prepare($sql);
  24. $stmt -> execute();
  25. echo '<ul>';
  26. foreach($stmt as $row)
  27. {
  28. echo '<li>'.$row['nazwa_branzy'].': '.$row['rodzaj_umowy'].': '.$row['login'].': '.$row['tresc'].': '.$row['lokalizacja'].': '.$row['date'].'</li>';
  29. }
  30. $stmt->closeCursor();
  31. echo '</ul>';
  32. //print_r($_GET);
  33. }
  34. }
  35. catch(PDOException $e)
  36. {
  37. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  38. }
  39. }
  40. }
nospor
Normalnie, zamiast jednej tablicy tworzysz dwie

nie: $wh[] = "b.nazwa_branzy = '{$_GET['nazwa']}'";
a:
$wh[] = "b.nazwa_branzy = ?";
$params[]={$_GET['nazwa']};

Druga tablica pojdzie do bindowania
Panicz74
Cytat
$wh[] = "b.nazwa_branzy = ?";
$params[]={$_GET['nazwa']};


Nie wiem czy dobrze rozumuję. Chodzi o coś takiego?:
  1. $wh[] = "b.nazwa_branzy = :nazwa_branzy";
  2. $stmt = $params[]=(':nazwa_branzy', $_GET['nazwa'], PDO::PARAM_STR);
  3. $sql -> bindValue($stmt);


questionmark.gif?
lukaskolista
Przeanalizuj mój post z tego tematu: http://forum.php.pl/index.php?s=&showt...t&p=1182715
Tam na końcu masz wywołanie metody execute($params), gdzie zgodnie z dokumentacją execute jako 1 argument podajesz tablicę parametrów.
Panicz74
Ok. W miarę załapałem o co chodzi. Faktycznie trochę mniej tego kodu teraz. Warto pytać. Ale zastanawia mnie teraz jak zbudować dobrze 2 z 3 parametrów bindValue.
Takie coś nie przechodzi:
  1. $stmt -> bindValue($mapping[$keys], $params, PDO::PARAM_STR);


Kod po zmianach:
  1. require_once 'connect.php';
  2. if(isset($_GET['nazwa']))
  3. {
  4. if($_GET['nazwa'] || $_GET['umowa'] || $_GET['lokalizacja'])
  5. {
  6. $mapping = array(
  7. 'nazwa' => 'nazwa_branzy',
  8. 'lokalizacja' => 'lokalizacja',
  9. 'umowa' => 'rodzaj_umowy'
  10. );
  11.  
  12. $where = array();
  13. $params = array();
  14. $wh = array();
  15.  
  16. foreach($_GET as $key => $value)
  17. {
  18. if(isset($mapping[$key]) && '' !== $value)
  19. {
  20. $where[] = $mapping[$key].' = :'.$mapping[$key];
  21. $params[':'.$mapping[$key]] = $value;
  22. }
  23. }
  24.  
  25. //2 SELECT
  26. if(!empty($where))
  27. {
  28. $where = 'WHERE '.implode(' AND ', $where);
  29. print_r($where);
  30. }
  31. try
  32. {
  33. $pdo = new PDO($pol, $user, $pass);
  34. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  35. if(isset($_GET['nazwa']))
  36. {
  37. $sql = 'SELECT b.nazwa_branzy, u.login, o.tresc, i.rodzaj_umowy, l.lokalizacja, o.date FROM ogloszenia AS o
  38. LEFT JOIN users u ON u.id_usera = o.id_usera
  39. LEFT JOIN branza b ON b.id_branzy = o.id_branzy
  40. LEFT JOIN umowa i ON i.id_umowy = o.id_umowy
  41. LEFT JOIN lokalizacja l ON l.id_lok = o.id_lok '.$where;
  42. $stmt = $pdo->prepare($sql);
  43. $stmt -> bindValue($mapping[$keys], $params, PDO::PARAM_STR);
  44. $stmt -> execute($params);
  45. //if($where = '') echo "Nic nie znaleziono";
  46. echo '<ul>';
  47. foreach($stmt as $row)
  48. echo '<li>'.$row['nazwa_branzy'].': '.$row['rodzaj_umowy'].': '.$row['login'].': '.$row['tresc'].': '.$row['lokalizacja'].': '.$row['date'].'</li>';
  49. $stmt->closeCursor();
  50. echo '</ul>';
  51. //print_r($_GET);
  52. }
  53. }
  54. catch(PDOException $e)
  55. {
  56. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  57. }
  58.  
  59. }
  60. }

lukaskolista
Ale po co bindujesz te parametry, skoro przekazujesz je w execute()? Wywal tą linijkę:
  1. $stmt -> bindValue($mapping[$keys], $params, PDO::PARAM_STR);

Dodatkowo używasz w niej zmiennej $keys, która nie istnieje.
Panicz74
Chciałbym to dobrze zrozumieć. Rozumiem, że teraz parametry są bindowane. Ale skąd execute wie, że mają być bindowane jako string?
lukaskolista
Bo są typu string. Jak będą intami, to będą bindowane jako inty. Skąd dokładnie wie? Nie chce mi się patrzeć w źródła, pewnie są tam jakieś ify.
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.