Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Najczęstsze błędy w PHP i antywzorce
Forum PHP.pl > Inne > Hydepark
Stron: 1, 2
Sephirus
Witam,

Ostatnio zastanawiałem się, jakie są często spotykane błędy, a raczej złe nawyki bądź po prostu bardzo nie wydajne lub nieoptymalne kawałki kodu jakie można znaleźć/napisać w PHP smile.gif Zbieram też antywzorce. Gdzież indziej mógłbym się o to zapytać niż nie tutaj smile.gif

Przykład wszystkim pewnie dobrze znany:

  1. $tablica; // powiędzmy 1000 elementów, jakichś obiektów, sporych
  2.  
  3. for($i = 0; $i < count($tablica); $i++) {
  4. // jakieś operacje
  5. }


Poprawna forma:

  1. $tablica; // powiędzmy 1000 elementów, jakichś obiektów, sporych
  2.  
  3. $elementsCount = count($tablica);
  4. for($i = 0; $i < $elementsCount; $i++) {
  5. // jakieś operacje
  6. }


@aras785 - masz rację dodałem poprawną formę smile.gif

Szukam podobnych konstrukcji, antywzorców programowania itp. Zależy mi na tym by były one proste smile.gif Nie szukam jakichś skomplikowanych 100 linijkowych kodów na dowód tego, że są źle napisane - chodzi mi o same zamysły smile.gif

Jeśli znacie jakieś piszcie proszę wink.gif
aras785
@Sephirus pisz również poprawne formy, ponieważ wejdzie początkujący i skąd będzie wiedział? Ps. w/w formę też używam czasami smile.gif

Ostatni pomagając na forum popełniłem błąd przy usuwaniu danych z bazy.

Błędy zapis:
  1. $q = mysql_query("SELECT id FROM test WHERE id=1 LIMIT 1--");
  2. if(mysql_num_rows($q)==1) {
  3. if(mysql_query("DELETE FROM test WHERE id=1 LIMIT 1--")) {
  4. echo('Usunięto!');
  5. } else echo('Błąd przy usuwaniu');
  6. }else echo('Nie ma takiego rekordu w bazie');


Poprawny:
  1. if(mysql_query("DELETE FROM test WHERE id=1 LIMIT 1--")) {
  2. echo('Usunięto!');
  3. } else echo('Błąd przy usuwaniu');


Objaśnienie: Jeśli nie znajdzie danego ID to go nie usunie i tak błąd wywali.

//poprawiłem. Kopiuj wklej i nie zauważyłem.
!*!
Formatowanie kodu też to obejmuje? wink.gif
Temat: Najczestsze bledy
Przejrzyj dział Oceny. Wypunktowane błędy są w prawie każdym wątku.
nospor
Cytat
a raczej złe nawyki
głupota i lenistwo.... dwa najczęściej spotykane na forum złe nawyki... smile.gif


A z bardziej przyziemnych rzeczy to np.
zapytania w pętli, gdzie główna pętla idzie z zapytania a w niej znowu pętla z zapytania smile.gif
A już szczyt szczytów zapytania w rekurencji....
Sephirus
@aras785 Twój kod nadal jest dla mnie nie zrozumiały (nie wiem po co ten SELECT na początku) - ale przeanalizuję to smile.gif

@!*! formatowanie (stricte) nie jest błędem a raczej niedbalstwem. Chyba, że ktoś nie lubi używać {} przy pętlach i instrukcjach a potem coś do nich dopisuje i się dziwi że zawsze działają ;P

  1.  
  2. if($costam === true)
  3. zrob_cos();
  4. zrob_cos_jeszcze();
  5.  
  6. // zamiast:
  7.  
  8. if($costam === true) {
  9. zrob_cos();
  10. zrob_cos_jeszcze();
  11. }
  12.  
  13. // itp...
  14.  


Temat z najczęściej pojawiającymi błędami jest mi znany - myślałem że wyłapie coś jeszcze osobno smile.gif Pomysł z przeglądaniem działu "oceny" jest jednak dobry - dzięki wink.gif tam na pewno coś znajdę wink.gif

@nospor

Na głupotę nie ma rady ;P ale nie mogę zakładać że każdy na nią cierpi - sam robię masę błędów a potem mi wstyd ;P ale nie uważam się za totalnego głupola smile.gif Co do lenistwa... true, true... :/

Zapytania w pętli to faktycznie częsta przypadłość biggrin.gif Dużo ludzi na początku nie wie co to JOIN, nie ma pojęcia co to relacyjność :/ no i przede wszystkim jak już napisali jakieś zapytanie i im działa, to już go nie ruszają i piszą nowe. Każdy z nas się chyba zgodzi, że na początku nauki termin "wydajność" jest nam obcy smile.gif

Ale zaintrygowałeś mnie tą rekurencją z zapytaniami!! biggrin.gif Jak gdzie tongue.gif w życiu tego nie widziałem - nie wiem nawet jak to napisać masz jakiś przykład? biggrin.gif
nospor
Cytat
Na głupotę nie ma rady ;P ale nie mogę zakładać że każdy na nią cierpi - sam robię masę błędów a potem mi wstyd ;P ale nie uważam się za totalnego głupola
Nie, nie o taką głupotę mi chodziło. Błędy przytrafiają się każdemu.

Cytat
Ale zaintrygowałeś mnie tą rekurencją z zapytaniami!! Jak gdzie w życiu tego nie widziałem - nie wiem nawet jak to napisać masz jakiś przykład?

A proszę bardzo, pierwszy przykład z brzegu:
Drzewko, tabela:
ID
ID_PARENT
.....

No i koleś pobiera rekurencyjnie całe drzewko z góry na dół. Najpierw wierzchołek, potem jego dzieci, potem dzieci dzieci i tak dalej....wszystko w ładnej niesamowidzie mulącej rekurencji smile.gif




Inny przykład kodów:
  1.  
  2. $zm = 1;
  3.  
  4. if ($zm == 1) echo 'jeden';
  5. if ($zm == 2) echo 'dwa';
  6. else echo 'trzy';

No i koleś się dziwi ze mu się wyswietal jeden i trzy, a przecież wyraźnie zmienna to 1


To jest też dobre:
  1. while($row=....){
  2. $dane = $row;
  3. }
  4. print_r($dane);
  5.  
  6. //EJ.... ludzie, czemu pobiera mi tylko ostatni rekord z bazy a nie wszystkie?!!!!! HELP!!!!

smile.gif

Albo to:
  1.  
  2. $row = mysql_fetch_array(....);
  3. while ($row = mysql_fetch_array(....)){
  4. print_r($row);
  5. }
  6.  
  7. //EJ.... ludzie, czemu pobiera mi wszystkie rekordy oprócz pierwszego?!!!! HELP!!!! 20 dni już nad tym siędzę!!!!!!!!!!!

wink.gif
Sephirus
Ok przyznam szczerze... z tym drzewkiem... że bym na to nie wpadł smile.gif co prawda od jakiegoś czasu sporo siedzę w SQL'u i obczajam jego wszelkie tajniki (poza administracją może bo to mi na nic) i może przez to wydaje mi się to dziwne, choć kiedyś kto wie - może i sam tak pisałem smile.gif

Ten drugi błąd to też bardziej pod formatowanie smile.gif I kurcze coś w tym jest - trzeba pisać "ładnie", estetycznie, zachowywać te wszystkie (jak to się na początku wydaje) głupie zasady tongue.gif ja osobiście nie przestrzegam tylko jednej. Czytałem parę propozycji jak pisać i formatować kod czy o nazewnictwie zmiennych itd. (także dla różnych frameworków itp.) więc mi się to utrwaliło i z tego wszystkiego mam swoją jedną wizję łączącą po kawałku ze wszystkiego ale nie przestrzegam zasady o tym, żeby linia miała <=80 znaków i nie znam osoby (osobiście) która tego przestrzega (oczywiście nie pisze ciurkiem ale jak coś mi wyskoczy poza 80 znaków to tak zostawiam po prostu).

Najgorszy błąd jaki mi się kiedyś przydarzył i naprawdę nie wiedziałem co z tym robić (pomogło dopiero przepisanie kodu od nowa haha.gif) wyglądał tak:

(może nie błąd, ale przeoczenie)
  1. for($i = 0; $i < $jakasWartosc; $i++); {
  2. // wykonaj jakiś kod
  3. }


Oczywiście ta pętla odwalała trochę roboty i to nie łatwej i nigdy nie chciała się wyświetlić :/ co bym nie robił tongue.gif Głupi średnik (który swoją drogą sam nie wiem jak się tam dostał zmarnował mi kupę czasu bo zdebugowałem cały skrypt praktycznie tongue.gif

EDIT: Te dwa ostatnie powalają nospor ale fakt nawet, że chyba na forum raz z czymś takim "pomogłem" tongue.gif
pyro
Tyle jest błędów i nieprawidłowych konstrukcji popełnianych przez ludzi, że ten cały wątek chyba nie ma celu, bo po pierwsze chyba miałby z 500 stron, a po drugie całe programowanie sprowadza się do poprawnego pisania algorytmów, więc ten temat jest tak jakby jak całe to forum goatee.gif
nospor
No i cała masa złego porównywania typów, nie rozróżnianie =, ==, ===

  1. if ($zm = 2) echo 'Czemu to mi sie zawsze wyświetla?';


  1. //funkcja xyz może zwracać false, oraz liczby od 0 wzwyż
  2.  
  3. if (xyz() == false) echo 'Źle';
  4. //Czemu mi ciągle wali Źle, skoro funkcja zwraca poprawny wynik?


  1. $i=0;
  2. while ($i <10){
  3. //tu masa kodu oprócz jednego najważniejszego $i++
  4. }
  5. //ej ludzie... zapetlam się.....


  1. while ($row = mysql_fetch_array($res)){
  2. //tu masa kodu
  3. //oraz
  4. $res = mysql_query();
  5. }
  6. //ej ludzie... pobiera mi tylko pierwszy rekord....

pyro
Ja do najgłupszych błędów, który popełniłem 30 sekund temu mogę zaliczyć zostawienie patelni na gazie, a potem robienie rzeczy przy komputerze. (ktoś mnie poprosił o pomoc z pewnym problemem natury programistycznej i zapomniałem o patelni). W kuchni pełno dymu, prawie spaliłem dom smile.gif . (ale omlet z cynamonem i tak wyszedł smaczny)
nospor
No i najważniejsze zło, jakie robią zarówno początkujący jak i ci, którzy uważają się za profesjonalistów:
  1. error_reporting(E_ALL ^ E_NOTICE);

Na to mi normalnie brak słów
Sephirus
@pyro Masz całkowitą rację niestety - to jest "rzeka", można by wymieniać w nieskończoność mimo to uważam, że dobrze jest ciągle wałkować ten temat smile.gif dlaczego?

Jestem sobie programistą, uważam że pozjadałem wszystkie rozumy smile.gif wchodzę na taki wątek i czytam, tu się uśmiechnę, tam się zaśmieje ale może, w pewnym momencie naglę przystanę i stwierdzę - o kurde... nie wiedziałem, że tak nie można smile.gif

No i poza tym chciałbym zebrać nieco materiałów na ten temat by się nimi podzielić z "nowymi" w pracy smile.gif i ich na to uczulić a sam wszystkiego nie pamiętam sad.gif

@nospor Co do operatorów to fakt, ale bardzo podobała mi się odpowiedź Erixa w temacie o najczęstrzych błędach, że "if($costam = 123)" to nie bug - tylko feature smile.gif To mi nasunęło pewną myśl: rzadko spotykam się z tym żeby ktoś to stosował :/

  1.  
  2. // w zamyśle jakas_funkcja zwraca albo null albo ciąg znaków
  3.  
  4. // zamiast (co jest dla mnie najlepsze)
  5. if($wynik = jakas_funkcja()) {
  6. echo 'Wynik prawidlowy i równy ' . $wynik;
  7. }
  8.  
  9. // jest
  10. $wynik = jakas_funckja();
  11. if($wynik) {
  12. echo 'Wynik prawidlowy i równy ' . $wynik;
  13. }
  14.  
  15. // no nie wspominając już o błędnym podejściu
  16. if(jakas_funkcja()) {
  17. echo 'Wynik prawidlowy i równy ' . jakas_funkcja();
  18. }


Czym może to być spowodowane że rzadko to ludzie stosują? Niewiedza? Gorsza przejrzystość kodu?
nospor
własnie w dziale przedszkole pojawił się problem z mojego posta wczesniej:
if ($zm = 'b')

Bład tak czesty, ze nawet podczas pisania posta pojawiają się takie tematy biggrin.gif
pyro
Cytat(nospor @ 14.01.2013, 14:16:41 ) *
No i najważniejsze zło, jakie robią zarówno początkujący jak i ci, którzy uważają się za profesjonalistów:
  1. error_reporting(E_ALL ^ E_NOTICE);

Na to mi normalnie brak słów


@nospor, bzdury Pan pleciesz! To nie jest żaden błąd, tylko konstrukcja, która choć nie zawsze stosowana z rozwagą, w wielu przypadkach usprawiedliwiona.
nospor
Cytat
Czym może to być spowodowane że rzadko to ludzie stosują? Niewiedza? Gorsza przejrzystość kodu?
Wg. mnie chodzi o przejrzystość. Sam tego nie stosuję choć dobrze wiem jak to działa smile.gif Wolę mieć przejrzysty kod niż super hiper featured wink.gif

@pyro to
if ($zm = 'b')
te tez nie jest błąd, tylko konstrukcja, rzadko stosowana z rozwagą wink.gif

Pomijam przypadki, gdy wyłączenie E_NOTICE jest usprawiedliwione. Mówię o sytujacji, gdy to w żaden sposób nie jest usprawiedliwione, np. gdy zaczynasz projekt od 0 i lecisz już z taką dyrektywą. Wówczas to jest fatalny błąd!

NIe chce mi się powtarzać, więc podam tylko linka
http://nospor.pl/notice-wyswietlac-czy-nie.html
pyro
Cytat(nospor @ 14.01.2013, 14:27:57 ) *
Pomijam przypadki, gdy wyłączenie E_NOTICE jest usprawiedliwione. Mówię o sytujacji, gdy to w żaden sposób nie jest usprawiedliwione, np. gdy zaczynasz projekt od 0 i lecisz już z taką dyrektywą. Wówczas to jest fatalny błąd!

NIe chce mi się powtarzać, więc podam tylko linka
http://nospor.pl/notice-wyswietlac-czy-nie.html


No to nie dopowiedziałeś i można było bardzo łatwo to źle zrozumieć. Trzeba było od razu dać link do artykułu, bo z tego co napisałeś można zrozumieć, że stosowanie tego jest zawsze złe.
Tuminure
Znajomość różnicy między && i AND, a także || i OR może się czasem przydać.

  1. $result1 = true AND false; // true
  2. $result2 = true && false; // false
  3. $result3 = false OR true; // false
  4. $result4 = false || true; // true
nospor
Cytat
No to nie dopowiedziałeś i można było bardzo łatwo to źle zrozumieć. Trzeba było od razu dać link do artykułu, bo z tego co napisałeś można zrozumieć, że stosowanie tego jest zawsze złe.
Dobrze, przepraszam. Niepotrzebnie przyjąłem, że każdy zrozumie o co mi chodzi smile.gif

@Tuminure o właśnie, ludzie często nie wiedzą do czego służą nawiasy oraz co to ważność operatora i często nie kumają czemu
2+3*4 nie jest równe 20 smile.gif
Oczywiście mówię tu w odniesieniu do mysql i php
Sephirus
Co do errorów to prawda... ale to jest celowe jakby nie patrzeć ;P zapominają o cudzysłowach itp. a potem jak im wyskakuje wiele notice'ów a mogą je ukryć (bo przecież wszystko działa) to sobie je olewają i ukrywają tongue.gif

U mnie w firmie jak raz włączyliśmy na głównym (starym) portalu logowanie do pliku wszystkich błędów z noticami włącznie to przez 30 minut plik z logiem miał 10GB smile.gif
nospor
Cytat
U mnie w firmie jak raz włączyliśmy na głównym (starym) portalu włączyliśmy logowanie do pliku wszystkich błędów z noticami włącznie to przez 30 minut plik z logiem miał 10GB
No właśnie, i weź potem w takiej ilości błędów znajdź człowieku jakąś literówkę.... nie ma szans.... i dlatego zawsze od początku trzeba pisać poprawnie. Gdy się tego nie zrobiło to potem już pozostaje jedynie na stałe wyłączenie błędów NOTICE i szukać przez 5 godzin jakis banalnych błędów, które z NOTICE znalazłoby się w dwie sekundy
Sephirus
Zgadzam się w 100%, niestety z takimi starymi bytami nie ma szans na poprawę - przyczyna jest prosta - albo brak "zasobów" by to zrobić - albo prosta, czysta "nieopłacalnośc" - jednak ktoś płaci za czas pracy programistów smile.gif

Zresztą po przeczytaniu Twojego nospor arta, umocniłem się jedynie w przekonaniu, że samo generowanie tych błędów zajmuje czas, toteż ich ukrywanie w dużej liczbie jedzie po wydajności - mnie to przekonuje do kwadratu smile.gif

@Tuminure ok może coś źle rozumiem ale nie widzę róznicy w PHP między AND a && i OR a ||?

Widzę jedynie pomiędzy AND/&& a & i odpowiednio OR/|| a |

Na pewno tak jest? smile.gif
thek
Ja przykładowo dzięki przeglądaniu kodów innych, przekonałem sie do "inwersji" w warunkach, czyli
  1. if(false == $zmienna)

która w razie czego zareaguje, gdyby mi się zamiast == wpisało samo =, co w debugu może przejść niezauważone.

Co do takich bardziej typowych błędów spotykanych, to oczywiście wszelkiego rodzaju gubienie się w zasięgu zmiennych spowodowane choćby używaniem globali lub zmiennych statycznych, których jednak część ludzi nie rozumie (inna sprawa, że global to zuo straszne). Często problemy z zapomnianym session_start. Chyba sztandarowy problem z wysyłaniem headersów przed funkcjami pokroju set_cookies wynikający albo z walnięcia entera przed <?php , czy zapisu strony ze znacznikiem BOM wink.gif

@Sephirus: Chodzi o priorytet operacji. && i || mają pierwszeństwo przed AND i OR, co może mieć odbicie w operacjach logicznych, gdy ktoś pomiesza oba rodzaje ze sobą wink.gif
Tuminure
Cytat
2+3*4 nie jest równe 20

Aż mi się przypomniało przeładowywanie operatorów z artykułu nonsensopedii dot. C++ tongue.gif

Do tego tematu w gruncie rzeczy pasuje też ten link: http://nonsensopedia.wikia.com/wiki/PHP

Cytat
ok może coś źle rozumiem ale nie widzę róznicy w PHP między AND a && i OR a ||?

Widzę jedynie pomiędzy AND/&& a & i odpowiednio OR/|| a |

Po prawej stronie podałem wyniki, które wskazałby var_dump na podanej zmiennej.
W podanym przez Ciebie linku w Example #1, również jest to co podałem.
Sephirus
@thek Tak ta "inwersja" jeśli chodzi o kolejność jest fajną sprawą i choć sam nie stosowałem - przekonałem się właśnie do tego.

Co do globali to mi się skojarzyło z register_globals od razu, choć jest to już temat może archaiczny to mimo wszystko nadal się go sporadycznie spotyka :| A już użycie funkcji extract aby go symulować bez żadnych zabezpieczeń i kontroli tego co przychodzi chociażby z GETów to już cofanie się w rozwoju a też to spotykam :/

EDIT: Właśnie znalazłem jeszcze jeden częsty błąd... Trzeba pamiętać by czyścić monitor :/ Właśnie kombinowałem czemu godzina pokazuje mi się jako

15;02

a nie

15:02

hmm... po znalezieniu odpowiedniej linijki, sprawdzeniu, i poruszeniu rolki myszki okazało się że to paproch :/
redeemer
W większości wypadków jako operator porównania lepiej stosować === zamiast == (http://gynvael.coldwind.pl/?id=492)
sowiq
Cytat(Sephirus @ 14.01.2013, 14:59:35 ) *
A już użycie funkcji extract...

Mój kolega powiedział kiedyś bardzo trafne porównanie - funkcja extract to rozrzutnik gnoju. I tyle w tym temacie smile.gif

A po kilku postach, które przejrzałem - zapomniane session_start, jakieś headery, zasięg zmiennych, pomylone = i ==... Kurde, ludzie, czy Wy dalej piszecie strukturalne PHP w Notatniku? O sesjach czy headerach zapomniałem w chwili kiedy pierwszy raz użyłem frameworka. Zasięg zmiennych? Może za czasów pisania strukturalnych skryptów, ale nie programowania w OOP. A o pomyłkach = / == każde średnio zaawansowane IDE informuje ostrzega na bieżąco.
Sephirus
Cytat
Mój kolega powiedział kiedyś bardzo trafne porównanie - funkcja extract to rozrzutnik gnoju. I tyle w tym temacie smile.gif


Czasem żałuję że nie ma przy postach na forum opcji z FB typu: LIKE smile.gif

Ale co do IDE - to ma pomóc a nie zastępować nam rozum smile.gif Dla doświadczonego programisty dobre IDE to skarb, nie dlatego że poprawia kod, ale że podpowiada i daje dużo przydatnych funkcjonalności. Jak ktoś jednak chcę się "nauczyć" programować to chyba jednak coś pokroju notatnik-plus byłby lepszy smile.gif a chodzi przecież o błędy początkujących wink.gif

@thek & @Tuminure tak to rozumiem - priorytet tak ale z zapisem:

  1. $result1 = true AND false; // true <-- TYM
  2. $result2 = true && false; // false
  3. $result3 = false OR true; // false <-- i TYM się nie zgadzam :)
  4. $result4 = false || true; // true


smile.gif
thek
@sowIq: co innego gdy korzystasz z Fw, a co innego gdy sam się zabrałeś za pisanie czegoś z celowym jego pominięciem, by narzut czasowy zlikwidować w ramach optymalizacji wąskich gardeł aplikacji.

Poza tym wiele osób nie zwraca uwagi, że funkcje php mogą zwrócić w normalnym trybie jako prawidłowy wynik 0 czy false, zaś jako nieprawidłowy null, a inne z kolei zwracają jako prawidłowe 0 i false, ale już nie null. Brak po prostu jednoznacznego określenia co jest prawidłowe, a co nie w tym języku. Wtedy:
  1. if ($zmienna == 0)

i mamy WTF bo false == 0, co nie zawsze jest tym o co chodziło wink.gif Pisząc od początku w jakimś Fw o wielu takich rzeczach się nie wie i nie myśli. Potem nadchodzi zmiana Fw i jest kaplica.

Headery? Nadal są gdy ktoś zapisze plik w UTF-8, ale nie zwróci uwagi jakim (stąd uwaga o BOM) albo przed <?php zdarzy mu się enter. Takich rzeczy Fw nie wyłapie.

Zasięg zmiennych? Nie każdy kod ma 5 linijek... Czasami metody z racji takich, a nie innych sytuacji mają ogromną złożoność i zdarza się, że w 5-6 zagnieżdżeniu ( niekoniecznie pętli, ale i struktury warunkowej) można się już na tym naciąć. Przykład Ci krótki dam, gdyż jakiś czas temu sobie skrobnąłem. Metoda ma znaczne complexity, ponieważ dane do niej przychodzące i obrabiane mogą być tablicą, ale i stringiem. Z kolei inny parametr może być raz stringiem, ale innym razem nullem wink.gif
  1. $bestHit = false;
  2. try{
  3. foreach($this->structure AS $main) {
  4. if(null !== $first && $first !== $main['name']) {
  5. continue;
  6. }
  7. foreach($main['childrens'] AS $nested) {
  8. if(is_array($name)) {
  9. if(in_array($name['base'], array_keys($nested['attr']))) {
  10. $this->property = array('main' => $main['name'], 'child' => $nested['name']);
  11. foreach($nested['attr'] AS $attr_name => $attr_data) {
  12. if( array_key_exists('params', $name) && $name['params'] == $attr_data['params'] ) {
  13. return $this;
  14. } else {
  15. $bestHit = true;
  16. }
  17. }
  18. }
  19. } else {
  20. if(in_array($name, array_keys($nested['attr']))) {
  21. $this->property = array('main' => $main['name'], 'child' => $nested['name']);
  22. return $this;
  23. } elseif ($name == $nested['name']) {
  24. $this->property = array('main' => $main['name'], 'child' => $nested['name']);
  25. return $this;
  26. }
  27. }
  28. }
  29. }
  30. if($bestHit) {
  31. return $this;
  32. }
  33. } catch (Exception $e) {
  34. throw tu_wyjatek_okreslony_przeze mnie(); // wiem co może mi rzucić, ale dla pewności łapię zakres Exception, a nie od niego pochodny
  35. }
  36. $this->property = array('main' => '', 'child' => '');
  37. return $this;
I powiedz mi, ile czasu zajęło by Ci zrozumienie co w tym się dzieje, w jakim celu i kiedy oraz co jest zwracane wink.gif Nie jest to kwestia sporzenia przez moment, a to tylko 30 linijek, będących całym ciałem jednej metody. Complexity tego kodu jest naprawdę duże, ale wynika ze struktury danych oraz danych wejściowych dla tej metody. Z tego nie wytniesz nawet linijki do metody prywatnej czy nie zrobisz klasy pomocniczej, która by to uprościła. Weź się walnij z returnem, miejscem inicjalizacji $bestHit lub przypisaniem danych do niej i masz problem z zasięgiem. A popatrz jak wiele miejsc ma ta metoda, by się pomylić, choć to tylko 2 zagnieżdżone foreach, tyle że ifów jest tu niestety wiele. I tak... Metoda ma napisany test, który ją całkowicie pokrywa, więc w razie czego wyłapię problem.
Sephirus
W sumie popieram przedmówcę ale (za pewne to babol jakiś) kod:

  1. if(in_array($name, array_keys($nested['attr']))) {
  2. $this->property = array('main' => $main['name'], 'child' => $nested['name']);
  3. return $this;
  4. } elseif ($name == $nested['name']) {
  5. $this->property = array('main' => $main['name'], 'child' => $nested['name']);
  6. return $this;
  7. }


nasuwa pytanie - czemu nie w jednym warunki i || ? smile.gif (wiem czepiam się)
sowiq
@thek, a co ma zasięg zmiennych do ilości i zagnieżdżenia pętli oraz złożoności kodu? Do rozwiązania ewentualnych problemów z tym związanych są - jak słusznie napisałeś - testy.

A problem z zasięgiem zmiennych bardziej kojarzy mi się z takim kodem:

  1. $var = 123;
  2.  
  3. function test1()
  4. {
  5. echo $var;
  6. }
  7.  
  8. function test2($var)
  9. {
  10. echo $var;
  11. test1();
  12. }
  13.  
  14. test1(); // wywali Warning
  15. test2(5); // wyświetli 5 a nie 123, po czym wywali Warning


Przykład niby oczywisty, ale już małe w zakłopotanie może wprowadzić niektórych dodanie
  1. global $var;

na początku funkcji test1().
Tuminure
Wychodziłem akurat z pracy, więc nie dokończyłem poprzedniego komentarza...

Cytat
@thek & @Tuminure tak to rozumiem - priorytet tak ale z zapisem:

Sęk w tym, że operator = jest wykonywany przed operatorem AND, a po operatorze &&. Analogiczna sytuacja występuje przy OR i ||.
W komentarzach umieściłem wersję z nawiasami, która powinna wyjaśnić w czym tkwi różnica.

W podanym przez Ciebie linku również jest to wytłumaczone w komentarzach (w Example#1) wink.gif.

http://codepad.org/KorJrKo7
Sephirus
Facepalm. ok @Tuminure masz całkowitą rację, od zupełnie innej strony do tego podszedłem smile.gif
thek
@sowiq: Przy złożonym kodzie bardzo łatwo się pogubić ze zmiennymi i tym co gdzie oraz jak inicjalizujemy, nadpisujemy lub zwracamy. Zauważ, że mógłbym przykładowo gdzieś w pętli zrobić break i mieć zmienną nadpisującą wewnątrz pętli inną, o zasięgu większym. Gdy takie sytuacje występują, bardzo łatwo się w tym pomylić. Pamiętajmy, że zasięg zmiennej to nie tylko zakres widoczności wnętrza funkcji, ale także przykładowo wnętrza pętli. To co wewnątrz pętli, poza nią też nie istnieje, czyli problem w stylu:
  1. for ($i = 0; $i < 10; ++$i) {
  2. echo $i;
  3. }
  4. echo $i;

@sephirus: taki zapis dla czytelności podczas etapu utrzymywania i konserwacji kodu smile.gif Łatwiej zauważyć innym, że raz mamy do czynienia ze stringiem, a innym razem z tablicą. Choć jak sam zauważyłeś, można to zbić do jednego, ale długość tego warunku przekroczyła by nie tylko sztywny limit 80 znaków, ale także miękki 120, a ja z racji czytelności staram się warunków nie "łamać" do następnej linii, choć na owo łamanie standardy pozwalają. No i narzędzia typu Sonar trochę potrafią Ci w kodzie potencjalnych pułapek wskazać wink.gif
Pilsener
Najgorsze są dla mnie błędy związane z wydajnością i bezpieczeństwem, typu:
  • niczym nie zabezpieczona rekurencja (tym gorsza, że uzależniona od parametrów z zewnątrz)
  • echowanie czego popadnie prosto z requesta lub tablicy $_SERVER, konstrukcje typu ... where id = $_GET['id'] i tak dalej
  • pobieranie wszystkiego z bazy jak leci albo wrzucanie do sesji jak leci - no bo wygodnie mieć wszystko w sesji, najlepiej całą bazę danych od razu wrzucić sobie do sesji, "by mieć taki kesz"
  • "dziś miejsce na dysku nie jest już problemem" - słyszałem to już x razy po czym po chwili dysk pada bo logujemy do plików pierdów bez liku


A jedne z najbardziej irytujących:
  • jednolita konfiguracja dla każdego środowiska, przez co regularnie commituje ustawienia lokalne albo po update wściekam się, że coś nie działa
  • brak jakichkolwiek komentarzy czy dokumentacji, najczęściej im paskudniejszy kod tym komentarzy jak na złość mniej
  • brak sprawdzania, czy coś się wykonało, założenie, że jak funkcja coś zwraca to z pewnością jest to spam, potem coś po prostu nie działa i sobie możesz szukać kilka dni no bo przecież "funkcja powinna wygenerować warninga chociaż jak jej kiepawo idzie"
  • używanie gdzie popadnie różnych fantastycznych wynalazków cudownego dziecka skryptu typu eval, goto, dynamiczne generowanie zmiennych - ekstrakcja to mały pikuś przy tym
  • obiektowość rozumiana tak, że np. co każde 100 linijek walniemy klasę a co 20 metodę, tworzenie jakiś dziwnych fabryk czy raczej młockarni obiektów które tworzą takie plewy, że człowiek tęskni za dynamicznym generowaniem zmiennych smile.gif
sowiq
Cytat(thek @ 14.01.2013, 22:13:48 ) *
To co wewnątrz pętli, poza nią też nie istnieje, czyli problem w stylu:
  1. for ($i = 0; $i < 10; ++$i) {
  2. echo $i;
  3. }
  4. echo $i;

Pudło kolego smile.gif Nie ten język. Za pętlą $i będzie miało wartość 10.

[edit]
Niektórzy mieli okazję pracować nad hinduskim kodem i im nie trzeba tłumaczyć smile.gif Najlepszy kwiatek jaki widziałem (napisany na 99% przez hindusa). Nie trzeba tego chyba komentować wink.gif
  1. eval('$i = ' . $j . ';');


Inna rzecz, jaka mnie wprawiła w zdumienie to jakiś portal społecznościowy z prywatnymi wiadomościami, do których można dodawać załączniki. Brak jakiejkolwiek walidacji sprawiał, że po załączeniu przez nadawcę pliku PHP, odbiorca zamiast go pobierać, wykonywał go na serwerze smile.gif

No i oczywiście temat rzeka - XSS. Należy pamiętać, że na modyfikację danych (dodawanie, usuwanie, update) należy pozawalać tylko requestom wykonywanym metodą POST.

[edit 2]
Jeśli chodzi o antywzorce, to znam jeden, który sam kiedyś z lubością (a raczej bez refleksji) praktykowałem. A chodzi o uzależnianie ilości wykonywanych zapytań SQL od ilości rekordów w bazie. Jak widać poniżej - im więcej rekordów będzie miała tabelka table1, tym więcej zapytań zostanie wykonanych na tabelce table2. Problem jest niezauważalny podczas przeklikiwania się przez aplikację z 10, 20, 50 rekordami w bazie. Ale problem może się zacząć pojawiać na serwerze produkcyjnym z dużo większą ilością danych.
  1. $records = $db->getRecords("SELECT * FROM table1");
  2.  
  3. foreach($records as $record)
  4. {
  5. $item = $db->getRecords("SELECT * FROM table2 WHERE category_id = ?", $record['id']);
  6. echo $item['name']; // jakakolwiek operacja
  7. }
  8.  
  9. /** a tutaj jak to powinno wyglądać **/
  10. $records = $db->getRecords("SELECT * FROM table1 t1 JOIN table2 t2 ON t2.category_id = t1.id");
  11.  
  12. foreach($records as $record)
  13. {
  14. echo $item['name']; // jakakolwiek operacja
  15. }
  16.  
  17. /** lub: **/
  18. $records = $db->getRecords("SELECT * FROM table1");
  19. $ids = array();
  20.  
  21. foreach($records as $record)
  22. {
  23. $ids[] = $record['id'];
  24. }
  25.  
  26. $records = $db->getRecords("SELECT * FROM table2 WHERE category_id IN (?)", implode(', ', $ids));
  27.  
  28. foreach($records as $record)
  29. {
  30. echo $item['name']; // jakakolwiek operacja
  31. }
Sephirus
@thek Rozumiem - ma to duży sens bo kod jest faktycznie bardziej czytelny, ja w tych miejscach jednak łamię warunki ;P stąd moje "czepianie się" wink.gif

Co do tej pętli to faktycznie sowiq ma rację - sam często wykorzystuję fakt, że w PHP zmienna użyta w pętli, po jej zakończeniu służy jeszcze do czegoś (dla jej ostatniej wartości).

@sowiq Oj tak - ten antywzorzec z liczbą zapytań, już tu zresztą wspominany, to naprawdę zmora niedoświadczonych z dużymi serwisami produkcyjnymi programistów, wszystko śmiga a potem na produkcji zaczyna się kombinowanie co jest nie tak że przymula smile.gif

Przypomniał mi się tutaj jeden fajny bajer jaki widziałem w kodzie młodego kolegi... i żeby nie było to autentyk smile.gif

  1. // jakieś operacje na PDO itd... środek jakiegoś modelu metoda zwracająca liczbę rek. w tabeli (coś z paginacją o ile pamiętam)
  2.  
  3. // ...
  4. $stmt = $pdo->query('SELECT * FROM tablica ');
  5. $results = $stmt->fetchAll();
  6. return count($results);
  7. // ...
  8.  


wink.gif
thek
@sowiq: w PHP to przejdzie, bo język ten ma swoje głupotki. W większości języków jednak zapis ten wywinie orła. Pisząc zaś o antywzorcach i błędach można wyjść poza jezyk, wskazując jego usterki, nielogiczności - w odniesieniu do innych znanych sobie języków. Bo niestety, ale język ten ma swoje upierdliwości, które widać patrząc z perspektywy innych. I nie chodzi mi o wydajność czy tego typu rzeczy, ale właśnie nielogiczność pewnych konstrukcji, struktur czy funkcji. Wspomniana niejednoznaczność wyniku funkcji, brak konsekwencji w kolejności parametrów (niektóre funkcje można wywołać raz tak, raz siak) czy problemy z zasięgiem zmiennych lokalnych (właśnie dlatego wspomniany) to przecież nie jedyne, które mogą wkurzać.

Co do zezwalania na modyfikacje tylko danym POST, to co to za różnica? Dane jedynie minimalnie trudniej spreparować. Po prostu nie możemy na pałę w linku podmienić parametru. Ale firebug, tamperdata lub podobne i zero problemów. To jest bardziej zalecenie projektowe.
sowiq
Cytat(thek @ 15.01.2013, 09:16:55 ) *
Co do zezwalania na modyfikacje tylko danym POST, to co to za różnica? Dane jedynie minimalnie trudniej spreparować. Po prostu nie możemy na pałę w linku podmienić parametru. Ale firebug, tamperdata lub podobne i zero problemów. To jest bardziej zalecenie projektowe.

Oczywiście, że to zalecenie projektowe, nikt Ci przecież nie każe na siłę tak robić. Ale jest to zalecenie dotyczące bezpieczeństwa. I nie chodzi tu o to, że "atakującemu" będzie trudniej wysłać takie zapytanie, bo to akurat pryszcz. Chodzi o XSS, o którym wspomniałem wcześniej. Jak na Twojej stornie masz jakąś dziurę pozwalającą na XSS, której nie zauważyłeś i ktoś doda Ci taki obrazek jak poniżej, to masz mały problem.
  1. <img src="/admin/post.php?act=deleteAll&confirmed=1" />

Chodzi o to, że zapytania POST nie da się wygenerować za pomocą prostego XSS. Musiałaby to być mega dziura pozwalająca na dodanie JS smile.gif

[edit]
Cytat(Sephirus @ 15.01.2013, 09:02:05 ) *
Przypomniał mi się tutaj jeden fajny bajer jaki widziałem w kodzie młodego kolegi... i żeby nie było to autentyk smile.gif

Kiedyś pracowałem przy rozbudowie jednego z większych portali obrazkowych w USA i widziałem identyczny kwiatek. Więc to nie jest domena tylko tych młodszych programistów i tylko tych mniejszych projektów wink.gif
thek
W przypadku XSS czy nie, zawsze się userowi robi "sandboxa", o czym wiele osób zapomina. Pozwalasz na jakieś akcje? Ogranicz to uprawnieniami tak mocno jak sie da. Taka przykladowa deletAll, nawet gdyby zadziałała, rąbnęła by nie całość bazy, ale maksymalnie to, do czego uprawnienia ma dany user. Niestety ten aspekt wielu ludzi "gubi" w czasie pisania aplikacji. Robią akcje napisane, mają uprawnienia, ale zapominają obie te rzeczy powiązać. To jest jeden z dość częstych błędów: brak lub szczątkowe zabezpieczenia na różnych poziomach.

Kwiatki takiego pokroju sam widziałem. Takie, gdzie z powodu braku spostrzegawczości potrafiło w czasie ładowania strony zahydrować przez Doctrine2 całą bazę - także.
O$iek
Co to znaczy "hydrować"? smile.gif
Sephirus
"hydrować" oznacza w jaki sposób zwracać rekordy/dane z bazy danych. Przykładowo można zwracać pojedynczy rekord z tabeli jako tablica asocjacyjna, zwykła, obiekt klasy stdClass, wartość pierwszej kolumny itd.

W PDO podobną rzeczą jest "FetchMode".
sowiq
Cytat(thek @ 15.01.2013, 13:14:20 ) *
Pozwalasz na jakieś akcje? Ogranicz to uprawnieniami tak mocno jak sie da. Taka przykladowa deletAll, nawet gdyby zadziałała, rąbnęła by nie całość bazy, ale maksymalnie to, do czego uprawnienia ma dany user.

Chyba nie zrozumiałeś co miałem na myśli i ogólnie jaka jest idea wykonywania takich operacji za pomocą POST, a nie GET.

Masz stronę z komentarzami. Ktoś dodaje w komentarzu obrazek z URL takim, jak Ci napisałem powyżej. Oglądasz komentarz zalogowany jako admin i bach bah!, posty poleciały. Ogląda to dowolny inny user (wstrzyknięty inny URL obrazka) i bach bah! - jego posty też poleciały. XSS nie polega na tym, że dowolny user bez uprawnień usunie coś z bazy danych. Polega na tym, że przeglądarka users z uprawnieniami wykonuje w tle requesta pod URL wstrzyknięty w ten lub inny sposób przez atakującego. Wydaje mi się, że mylisz te dwa pojęcia.

Zezwalanie na modyfikację danych tylko za pomocą zapytań POST znacznie ogranicza możliwości takiego ataku. Oczywiście można też robić jakieś jednorazowe tokeny wysyłane w GET itp., ale specyfikacja protokołu HTTP mówi jasno - odczyt danych = GET, modyfikacja danych = POST.
Sephirus
Cytat(sowiq @ 15.01.2013, 14:14:48 ) *
Polega na tym, że przeglądarka users z uprawnieniami wykonuje w tle requesta pod URL wstrzyknięty w ten lub inny sposób przez atakującego.


Tak BTW to to bardzo kojarzy się z XSRF (CSRF) - który często jest celem dla samego XSS (choć może działać bez niego oczywiście).
O$iek
Cytat(Sephirus @ 15.01.2013, 14:09:30 ) *
"hydrować" oznacza w jaki sposób zwracać rekordy/dane z bazy danych. Przykładowo można zwracać pojedynczy rekord z tabeli jako tablica asocjacyjna, zwykła, obiekt klasy stdClass, wartość pierwszej kolumny itd.

W PDO podobną rzeczą jest "FetchMode".

Dzięki.
ano
Cytat(sowiq @ 15.01.2013, 09:39:11 ) *
Oczywiście, że to zalecenie projektowe, nikt Ci przecież nie każe na siłę tak robić. Ale jest to zalecenie dotyczące bezpieczeństwa. I nie chodzi tu o to, że "atakującemu" będzie trudniej wysłać takie zapytanie, bo to akurat pryszcz. Chodzi o XSS, o którym wspomniałem wcześniej. Jak na Twojej stornie masz jakąś dziurę pozwalającą na XSS, której nie zauważyłeś i ktoś doda Ci taki obrazek jak poniżej, to masz mały problem.
  1. <img src="/admin/post.php?act=deleteAll&confirmed=1" />

Chodzi o to, że zapytania POST nie da się wygenerować za pomocą prostego XSS. Musiałaby to być mega dziura pozwalająca na dodanie JS smile.gif

[edit]

Kiedyś pracowałem przy rozbudowie jednego z większych portali obrazkowych w USA i widziałem identyczny kwiatek. Więc to nie jest domena tylko tych młodszych programistów i tylko tych mniejszych projektów wink.gif


Kluczem problemu nie jest XSS a CSRF, którego w żaden sposób nie zabezpieczy to, że akceptujemy tylko requesty POST. (Bardzo łatwo wykonać "automatycznego" POSTA - wystarczy jedna linijka JSa ;-) )
Mała autoreklama - więcej info na: http://tinyurl.com/boqerob ;-)
sazian
Cytat(Pilsener @ 14.01.2013, 23:09:36 ) *
używanie gdzie popadnie różnych fantastycznych wynalazków cudownego dziecka skryptu typu eval, goto, dynamiczne generowanie zmiennych - ekstrakcja to mały pikuś przy tym

a widziałeś skrypt forum vbulletin ? tam co chwilę masz eval na jakiś kod pobierany z bazy. Edytowanie tego to istna masakra szczególnie jak trafisz na funkcje której połowa kodu jest zapisana w bazie

pamil
+ do wykonania CSRF wcale nie potrzebujemy dziury XSS w atakowanej stronie smile.gif
Lysiur
Wiecie co, od samego początku powstania tego tematu śledziłem jego rozwój. Jak go tylko zobaczyłem pomyślałem "oho, będzie się działo" (ironia). Niestety dalej podtrzymuje takie stanowisko w tej sprawie smile.gif Temat raczej nie jest specjalnie pomocny, nawet jak na temat for fun. Sorry, jestem zgorzkniałym zgredem, ale takie jest moje odczucie. Myślę, że bardziej konstruktywnym tematem, byłoby zestawienie praktycznych, poprawnych i przydatnych postów (nie antywzorców). Czemu tak? Często jest zdarza się (zwłaszcza na początku), że zobaczymy jakiś przykład (z głupawym antywzorcem) i go stosujemy - bo nam działa. Mimio, iż doskonale zdajemy sobie sprawę, że nie jest to dobre i wmawiamy sobie, że później to poprawimy. Początkujący chwytają się wyszystkiego by tylko działało i poczuli się lepiej, że coś udało im się stworzyć konstruktywnego.

Przeglądając posty z tego tematu, nawet nie natknąłem sie (a jeśli tak - to zwracam honor) na tak banalny antywzorzec jak najprosztsze generowanie całej storny przy pomocy instrukcji echo

  1. echo '<table>
  2. <tr><td>'.$zmienna.'</td></tr>
  3. </table>';


A często się zdarzają się w postach takie perełki.

Czy nie lepszym tematem byłby zbiór "Przydatnych i ciekawych rozwiązań", który mógłby zawierać jakieś przydatne fragmenty, które uczyły by początkujących dobrych praktyk i sprawdzonych metod? Opis rzadko używanych funkcji, z których warto skorzystać w takiej - a nie innej sytuacji. Krótkie omówienie najprostszych wzorców obiektowych w sposób praktyczny, z ew. linkami do dalszej lektury. W końcu każdy ma inne podejście do wzorców i temat ten mógłby być ciekawym polem do dyskusji. Podobnie jak sposób w jaki przystępujęcie do projektowania danego zagadnienia, na co zwracacie uwagę i czego używacie - maind mapów, etc.

wydaje mi się, że takie tematy byłby częściowo for fun, jak i przydatne dla każdego z nas. Dziś takiego tematu nie zacznę - bo nie mam na to czasu, ale myślę, że z czasem rozpocznę taki cykl "dłuższych" tematów.

Pozdrawiam i dobrej nocy!
Sephirus
@pamil Masz rację - tak jak wcześniej pisałem CSRF nie wymaga XSS ale jeśli mamy możliwość jakiegoś XSS z Javascript to CSRF staje się dużo bardziej niebezpieczny.

@Lysiur Masz rację i nie masz smile.gif To prawda że temat to jeden wielki chaos ale powiem Ci szczerze, że sam czegoś się nauczyłem lub sobie przypomniałem a nie uważam się za początkującego. Pisząc jak coś robić a nie czego nie robić to dobra praktyka, niekiedy jednak dobrze jest spojrzeć na to z drugiej strony. Zresztą - jeśli opisujemy jakiś zły nawy, błąd itp. to automatycznie dajemy info o tym jak to powinno być zrobione poprawnie.
!*!
Przypomniała mi się rzecz która zdarza się dość często, mianowicie masowa produkcja niepotrzebnych zmiennych, lub złe odwołania do funkcji.

  1. include 'cfg.php';
  2. if(isset($config))
  3. {
  4. $this->cfg = $config;
  5. }
  6.  
  7. echo $this->cfg['host'];
  8.  
  9. //plik cfg.php:
  10. $config = array();


Zamiast:
  1. $this->cfg = include_once 'cfg.php';
  2. //plik cfg.php:
  3. return array();


Jeśli zaś chodzi o funkcje...
  1. function Foo(){ // tu jakaś super skomplikowana funkcja która liczy i liczy i liczy... aż w końcu zwraca wynik
  2.  
  3. // a tu taki psikus
  4. while()
  5. {
  6. echo Foo();
  7. }
  8.  
  9. // zamiast
  10.  
  11. $r = Foo();
  12. while()
  13. {
  14. echo $r;
  15. }


Oczywiście są to w miarę proste przykłady, ale one potrafią się rozbudować szczególnie przy fantazji początkujących.
Do tego dojdzie jeszcze upieranie się że funkcje mają przewagę nad całym OOP biggrin.gif
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.