Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Ocena prostego skryptu
Forum PHP.pl > Inne > Oceny
lysiu
Zależy mi na ocenie mojego skyptu. Uczę się i chcę wiedzieć co robię źle. Skrypt już ściągneło 14 osób ale nikt nawet nie raczył mi powiedzieć czy działa, czy może jest całkiem do kitu. Skrypt jest bardzo mały i jego przeanalizowanie nie zajmie wam dużo czasu. To mój drugi skrypt po totalnie beznadziejnej księdze gości, którą napisałem jakiś czas temu ;-) [źródło]. Planuję już następny bardziej skomplikowany skrypt (so beware!!!) ;-))))

Dzięki!!

Edit [przykład użycia]:
  1. $l = polodm_rozloz_liczbe( 3456 );
  2. $s = polodm_zamien_na_slowa( $l );
  3. $k = polodm_odmieniaj( $l );
  4.  
  5. echo $s, ' ', $k; // wynik: Trzy tysiące czterysta pięćdziesiąt sześć komentarzy
Cypherq
Może się mylę, ale jak już używasz funkcji to nie lepiej opakować je w klasę i dodać pole publiczne zamiast bawić się globalami?
Kildyt
- poczytaj o programowaniu obiektowym,
- global'e nie są najlepszym rozwiązaniem,
- polskie nazwy - po tym poznasz amatora

To tak na pierwszy rzut oka.
lysiu
ok, dzięki. Dzisiaj to poprawie czarodziej.gif
patryczakowy
Cytat(Kildyt @ 3.10.2009, 13:30:38 ) *
- polskie nazwy - po tym poznasz amatora


Gratuluje poczucia humoru
darko
Logiki, ani wydajności nie analizowałem, ale

dobrze:

-utrzymanie jednolitej konwencji nazewniczej funkcji: oddzielające podkreślniki (_) i rozpoczynanie nazw od "polodm"

żle:

- lepiej, wygodniej, estetyczniej i przejrzyściej użyć klas(y) i metod zamiast grup funkcji
- polskie nazewnictwo funkcji (standardem jest używanie języka angielskiego w kodzie, nawet jeśli odbiorcą skryptu jest Polak/Polka)
- jeśli chcesz sprawdzić czy zmienna jest NULL to robi się to tak:

if(is_null($var)) {

}

- zamiast zwracać z funkcji typ NULL lub string "(NULL)" - zwróć wartość false
- te drugie nawiasy nie są potrzebne, wynik operacji modulo i tak zostanie przypisany do zmiennej $liczba: if( $liczba > 9999 ) $liczba = ($liczba%10000);


To tak pobieżnie, już na początku napisałem, że nie analizowałem ani logiki ani pod kątem "co by można tu zrobić szybciej i krócej".

Życzę powodzenia w dalszej nauce PHP
Cypherq
Cytat(patryczakowy @ 4.10.2009, 16:21:26 ) *
Gratuluje poczucia humoru


A nie? Jak pracujesz w firmie i robisz jakiś system internetowy to gdy zajmie się nią zagraniczna grupa, szybko uczy się Polskiego. Co więcej, jak bierzesz system zrobiony przez Duńczyków, to uczysz się Duńskiego. No i CakePHP, on pewnie też był pisany po polsku.
lysiu
Dzięki za wszystkie poprawki, niektóre wprowadziłem. Miałem trochę problemu z przejściem na język obiektowy, ale sobie poradziłem. poprawione

Ale mam inny problem. Ponieważ nie byłem pewien czy moje zmienianie daty dobrze działa, dodałem opcję by ten ficzer wyłączyć.

Metoda (kod, przy którym się zapętla zaznaczyłem wykrzyknikami):
  1. // Niewiele zmienia
  2. public function data( $d = '' )
  3. {
  4. if( get_option( 'polodm_uzyj_daty' ) == 'no' )
  5. {
  6. // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    !!!!!!!!!!!!
  7. // proszę mi wytłumaczyć, czemu to się zapętla w nieskończność
  8. echo apply_filters('the_time', get_the_time( $d ), $d);
  9. // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    !!!!!!!!!!!!
  10. echo "To się nigdy nie wyświetli!";
  11.  
  12. // Narazie jest tak w rep:
  13. // echo $d;
  14.  
  15. return;
  16. }
  17.  
  18. $id = $GLOBALS['id'];
  19.  
  20. // narazie, więc godzina jest po prostu wyświetlana normalnie
  21. if( preg_match( "/\d*:\d*.?.?.?/", $d ) )
  22. {
  23. echo $d;
  24. return;
  25. }
  26.  
  27. $pat_daty = get_option( 'polodm_pat_daty' );
  28. $uzyj_custom = trim(get_option( 'polodm_uzyj_custom'));
  29. $custom = stripslashes(get_option( 'polodm_custom' ));
  30.  
  31.  
  32. $rok = get_post_time('Y', false, $id, true);
  33. $miesiac = get_post_time('n', false, $id, true);
  34. $dzien = get_post_time('j', false, $id, true);
  35. $go = '-go';
  36.  
  37. $sprawdz = str_replace(' ', '', $custom );
  38. $sprawdz = str_replace("\t", '', $sprawdz);
  39.  
  40.  
  41. $pat_daty = str_replace( '[dzien]', $dzien, $pat_daty );
  42. $pat_daty = str_replace( '[rok]', $rok, $pat_daty );
  43. $pat_daty = str_replace( '[miesiac]', $this->miesiace[$miesiac], $pat_daty );
  44. $pat_daty = str_replace( '[]', $go, $pat_daty );
  45.  
  46. if( $uzyj_custom == 'yes' && !empty($sprawdz) )
  47. {
  48. $W_POKAZ = FALSE;
  49. $W_DATA = TRUE;
  50.  
  51. if( @eval( $custom ) !== FALSE )
  52. {
  53. if(!empty($wynik)) $pat_daty = $wynik;
  54. }
  55. }
  56.  
  57. echo $pat_daty;
  58. }
  59.  


Funkcję rejestruje w wordpress:
  1. add_action ( 'the_time', array( &$this, 'data' ), 1, 5 );



A błąd dostaje taki:
Kod
Fatal error: Maximum execution time of 30 seconds exceeded in E:\Programy\WebServ\httpd\vhosts\wp\wp-includes\functions.php on line 118


Nie wiem, czy potrzebna jest znajomość wordpressa by to zdiagnozować smile.gif
Z góry dziękuję, i pozdrawiam.
Cypherq
Z jakichś powodów Twoja funkcja przekracza maksymalny czas który dostaje od serwera na wykonanie się. Przetestuj ją (czas jej wykonania w różnych momentach, żeby znaleźć dziurę) funkcją microtime()
lysiu
Cytat(Cypherq @ 8.10.2009, 22:59:15 ) *
Z jakichś powodów Twoja funkcja przekracza maksymalny czas który dostaje od serwera na wykonanie się. Przetestuj ją (czas jej wykonania w różnych momentach, żeby znaleźć dziurę) funkcją microtime()


Przecież napisałem kiedy i gdzie się zapętla. Tylko nie wiem dlaczego, na #wordpress też nie wiedzą, a pewnie błąd jest zbyt oczywisty by go dostrzec smile.gif
Cypherq
Oj, bo takie rzeczy się pisze w poście a nie komentując kod.

http://wordpress.org/tags/apply_filters
http://codex.wordpress.org/Function_Reference/apply_filters
http://codex.wordpress.org/Function_Reference/get_the_time

Masz i grzeb, że tak powiem. Wtedy naprawdę się czegoś nauczysz i będziesz miał z tego satysfakcję.
lysiu
tak doładniej to zawiesza się na:
  1. $dateformatstring = preg_replace( "/([^\\\])a/", "\\1" . backslashit( $datemeridiem ), $dateformatstring );


;-)

przydało by się gdb, a pozatym jak to się może zawiesić, tego nie rozumiem.
Cypherq
A na jakim tekscie operuje ta funkcja?
lysiu
dodałem echo przed zapętlającą się funkcją:
  1. echo '<br />$datemonth_abbrev=\''.$datemonth_abbrev.'\',<br /> $dateformatstring=\''.$dateformatstring.'\';<br /><br />';
  2. $dateformatstring = preg_replace( "/([^\\\])a/", "\\1" . backslashit( $datemeridiem ), $dateformatstring );


i oto część outputu: http://pastie.org/648233
a tu jak chcesz masz cały, wczytuje się dokładnie 30s: http://irci.mine.nu:666/
Cypherq
Łe? Tego kodu nie ma w kodzie wtyczki który wkleiłeś, wklej cały kod.
lysiu
heh winksmiley.jpg To jest funkcja wordpress; wp-include/functions.h -> http://xref.yoast.com/2.8/nav.html?wp-incl...source.html#118 <-- linia 118, przed tą linią dodałem echo. Wcześniej wkleiłem swój kody przy którym się zawiesza, i napisałem jaki jest błąd, wątpię, żeby to był problem wordpress'a, raczej moja funkcja, wywołuje go za dużo razy, przynajmniej tak mi się wydaję.

jak widać w linku http://irci.mine.nu:666/ $dateformatstring cały czas się rozrasta, i gdyby nieograniczenia czasu wykonywania skryptu, rozrastało by się w nieskończoność smile.gif
thomson89
Cytat(Kildyt @ 3.10.2009, 13:30:38 ) *
- polskie nazwy - po tym poznasz amatora


Ja siedzę w php już kilka lat i nadal używam polskich nazw, bo w końcu jestem polakiem!
Cypherq
Piszesz dla siebie - ok. I to nazywamy programowaniem amatorskim. Gdybyś chciał stworzyć coś większego, niekoniecznie dla firmy, dla klienta, po prostu napisać klasę, którą chciałbyś się podzielić ze światem, pewnie napiszesz ją po polsku, co? A ludzie z innych państw, chcący przeanalizować działanie Twojej klasy, lub ją zmienić, będą musieli się nauczyć polskiego, tak?
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.