prośba o krytykę kodu |
prośba o krytykę kodu |
12.06.2013, 19:33:50
Post
#1
|
|
Grupa: Zarejestrowani Postów: 125 Pomógł: 9 Dołączył: 25.05.2013 Ostrzeżenie: (0%) |
jako, że nie jestem "PRO" w PHP chciałbym swoje ewentualne błędy korygować na bieżąco, czy mógłby ktoś rzucić okiem na mój kod i go skrytykować conieco?
ewentualnie na gist: https://gist.github.com/mprz/6cce0e6dd51e4796209c -------------------- |
|
|
12.06.2013, 19:39:57
Post
#2
|
|
Grupa: Moderatorzy Postów: 4 069 Pomógł: 497 Dołączył: 11.05.2007 Skąd: Warszawa |
DOMAIN_OK, DOMAIN_EXPIRING, gdzie ty to deklarujesz?
Trigger_error <- co to za cudo z średniowiecza? Czemu każda metoda wywołuje DB::getConnection(); ? Daj to w konstruktorze. Private $_expired i już wiem że nici z wygodnego dziedziczenia. |
|
|
12.06.2013, 21:23:44
Post
#3
|
|
Grupa: Zarejestrowani Postów: 125 Pomógł: 9 Dołączył: 25.05.2013 Ostrzeżenie: (0%) |
deklaracje są w config.inc.php, ale już załapałem że lepiej będzie trzymać w klasie
trigger_error jest tam w tylko w fazie 'development', później zmienię na logowanie błędów do pliku co do prywatnych zmiennych - przecież do wszystkich są gettery, jeśli nie planuję rozszerzalności klasy nie mógłbym ich zostawić jako private? znam różnicę między private i protected edit: nowa wersja, zmienione private->protected, dodane stałe, jedna zmienna dla połączenia PDO https://gist.github.com/mprz/6cce0e6dd51e4796209c Ten post edytował dżozef 12.06.2013, 19:59:33 -------------------- |
|
|
12.06.2013, 22:52:19
Post
#4
|
|
Grupa: Zarejestrowani Postów: 1 447 Pomógł: 191 Dołączył: 26.03.2008 Ostrzeżenie: (0%) |
Czemu nazwy składowych zaczynają się od znaku _ ? Czy to nie jest naleciałość po php4 gdzie nie było jeszcze private ? Czy nadal jest to potrzebne ?Nie wiem czy to błąd, czy nie. Po prostu pytam.
Masz sporo komentarzy. Według Wujka Boba, komentarz to "zło konieczne" gdy kod nie opisuje sam siebie. Czy więc zamiast metody o nazwie delete, nie lepiej nazwać ja deleteDomian ? Podobnie metoda numExpiring() z komentarzem "returns number of expiring domains", mogła by się obejść bez komentarza, jeśli nazwalibyśmy ją getNumExpiringDomains() Ten post edytował peter13135 12.06.2013, 22:56:19 -------------------- :)
|
|
|
12.06.2013, 23:04:09
Post
#5
|
|
Grupa: Zarejestrowani Postów: 125 Pomógł: 9 Dołączył: 25.05.2013 Ostrzeżenie: (0%) |
dzięki za rady!
- podkreślenie służy mi temu, że od razu wiem, która zmienna jest public a która private/protected. podejrzałem to gdzieś i zdaje się mi służyć - naczytałem się troche o konwencji nazw dla PHP (i OOD wogóle) i pamiętam, że duży nacisk położony był na nie dołączanie do nazw metod nazwy klasy. czyli:
a nie
gdzie druga wersja niepotrzebnie powiela Domain wydaje mi się to rozsądnie pomyślane, dlatego robię to w ten sposób -------------------- |
|
|
12.06.2013, 23:10:23
Post
#6
|
|
Grupa: Zarejestrowani Postów: 1 447 Pomógł: 191 Dołączył: 26.03.2008 Ostrzeżenie: (0%) |
Fakt, z tym się zgodzę. Nawet nie spojrzałem na nazwę klasy. W takim razie, komentarz "// delete domain" dla metody delete w klasie domain moim zdaniem jest zbyteczny, bo nie mówi niczego więcej ponad to, co wnioskujemy po nazwie klasy i metody. Być może czepiam się pierdół. Po prostu kod jest (moim zdaniem) na tyle dobry, że czepiam się tego co się da
Według Wujka Boba współczesne IDE są na tyle rozbudowane, że takie podkreślenia lub inne prefixy są zbyteczne. -------------------- :)
|
|
|
12.06.2013, 23:15:41
Post
#7
|
|
Grupa: Zarejestrowani Postów: 125 Pomógł: 9 Dołączył: 25.05.2013 Ostrzeżenie: (0%) |
Po prostu kod jest (moim zdaniem) na tyle dobry, że czepiam się tego co się da wow, dzięki to moja pierwsza aplikacja Według Wujka Boba współczesne IDE są na tyle rozbudowane, że takie podkreślenia lub inne prefixy są zbyteczne. bardziej chodzi mi o to, że to JA widzę, która zmienna jest/nie jest publiczna -------------------- |
|
|
13.06.2013, 08:40:11
Post
#8
|
|
Grupa: Moderatorzy Postów: 4 069 Pomógł: 497 Dołączył: 11.05.2007 Skąd: Warszawa |
Tyle razy były już o tym dyskusje. Zaczynanie metod private/protected od _ ma sens i jest nadal bardzo przydatne. |
|
|
13.06.2013, 08:43:33
Post
#9
|
|
Grupa: Zarejestrowani Postów: 125 Pomógł: 9 Dołączył: 25.05.2013 Ostrzeżenie: (0%) |
uaktualnienie: kilka funkcji nie sprawdzało czy zwracana wartość została zainicjowana. dodałem parę warunków w kodzie, i zwracane jest -1 jeśli $_result jest unset()
-------------------- |
|
|
13.06.2013, 09:11:00
Post
#10
|
|
Grupa: Moderatorzy Postów: 4 069 Pomógł: 497 Dołączył: 11.05.2007 Skąd: Warszawa |
czemu -1? o.O Zwracaj false.
Na szybko klepnąłeś, nie chciało ci się ochłonąć i pomyśleć. Zamiast tego:
Daj np. to:
Ewentualnie jeśli zależy ci na zachowaniu $return:
Jeśli robisz lokalne zmienne takie jak $result, nie ma sensu dawać _ czyli $_result. Znak _ dajesz dla private/protected. |
|
|
13.06.2013, 10:15:17
Post
#11
|
|
Grupa: Zarejestrowani Postów: 195 Pomógł: 14 Dołączył: 12.01.2006 Skąd: Gotham City Ostrzeżenie: (0%) |
- podkreślenie służy mi temu, że od razu wiem, która zmienna jest public a która private/protected. podejrzałem to gdzieś i zdaje się mi służyć To źle ci służy. Wszystkie dane( pola ) dobrze zaprojektowanej klasy powinny być prywatne ( enkapsulacja danych ), więć nie ma żadnego powodu by dodawać do nich podkreślink skoro wszystkie są prywatne. Dane chornione czy publiczne to są bardzo specjalne przypadki. Dostęp do danych klasy powinien odbywać się tylko przez metody dostępowe i to one są najwyżej chronione. Interfejs jest publiczny.Podkreślnik przed zmiennymi prywatnymi to zboczenie programistów javy ponieważ tam można odwołać się do pola klasy beż this w jej obrębie i nie widać czy to zmienna lokalna czy klasy na pierwszy rzut oka. Pozatym zmiennych lokalnych nie powinno się używać tylko zapytania. Zastępujemy zmienną tymczasową zapytaniem. Twoja klasa jest odpowiedzialna za 2 rzeczy czyli o jedną za dużo. Widze tu... póki jeszcze nie zarzuciłem bielunia to widze... obiekt dziedziny domain i utrwalanie go. 2 klasy bym tu minimum widział oczywiscie domain i np domain_table. Warstwa dziedziny i warstwa utrwalania. Domain bedzie delegować do domain_table. domain_table będzie polem klasy domain. Na pewno musisz zastosować te przekształcenia tak na oko * Rozdzielenie Zapytania i Modyfikacji * Zastąpienie odwołania do zmiennej zapytaniem * Ekstrakcje klasy ( z domain wyciagnij np domain_table ) * Ekstrakcje metody ( podzialkuj dlugie metody ) * Przeniesienie metody ( przenies do domain_table z domain ) Zastosować jakiś wzorzec do mapowania realcyjno-obiektowego * Brama danych tabeli * Brama danych wiersza * Rekord aktywny * Data mapper to by była w tym wypadku lekka przesada. Dobrze że używasz instrukcji preparowanych i nazwanych symboli zastępczych ci którzy tak nie robią robią po prostu siare. Każdy kto nie czytał tych ksiązek jak i welu innych klasyków, a programuje powinien się wstydzić i kajać. Refaktoryzacja ulepszanie struktury istniejacego kodu Architektura systemow zarzadzania przedsiebiorstwem wzorce projektowe Wracam do szukania bielunia obok firmy może coś znajde. Pissss joł. Ten post edytował emp 13.06.2013, 11:08:33 -------------------- Temat zamykam i przenoszę do Bangladeszu.
To jest wiadomość śmierci jeśli ją czytasz to znaczy że pozostało ci 30 sekund życia, więc lepiej zacznij się modlić. |
|
|
13.06.2013, 21:32:17
Post
#12
|
|
Grupa: Zarejestrowani Postów: 1 447 Pomógł: 191 Dołączył: 26.03.2008 Ostrzeżenie: (0%) |
Cytat Podkreślnik przed zmiennymi prywatnymi to zboczenie programistów javy ponieważ tam można odwołać się do pola klasy beż this w jej obrębie i nie widać czy to zmienna lokalna czy klasy na pierwszy rzut oka. Osobiście w javie nie spotkałem się z takimi zmiennymi. Metoda teoretycznie powinna być bardzo mała, a współczesne IDE tak kolorują składnie, by wiadomo było, czy zmienna jest składową klasy, czy lokalną w metodzie - dlatego nadal nie widzę powodu, by programiścy javy (i czemu akurat javy? w cpp też nie trzeba używać thisa) używali tego podkreślenia. Pewnie za mało kodu w javie widziałem albo oglądam tylko ten lepszy kod. Natomiast podkreślenia w zmiennych prywatnych to bardzo częste zachowanie programistów PHP4 gdzie każda zmienna była publiczna (słowo kluczowe var), a dodanie podkreślenia na początku miało sygnalizować - tą zmienną traktuj jak prywatną, bo ona byłaby prywatna gdyby tylko php4 na to pozwoliło. Cytat i zwracane jest -1 jeśli $_result jest unset() A czy tutaj nie lepiej skorzystać z wyjątków ? Wiem, że wyjątki w php to takie troche sztuczne są (choćby dlatego, że wyjątkami nie zostały objęte wbudowane funkcje), ale jak już mówimy o czystym kodzie, to wyjątki są po to, żeby ułatwić pisanie czystego kodu. Cytat Tyle razy były już o tym dyskusje. Zaczynanie metod private/protected od _ ma sens i jest nadal bardzo przydatne. Wybacz, nie widziałem ani jednego wątku. W książce, którą czytałem używanie podkreślenia było krytykowane. Chętnie poznam inne opinie na ten temat, więc jakimś linkiem nie pogardzę
takie same komentarze, dla dwóch metod ? Chyba cośtu jest nie tak. Według Wujka Boba, takie warunki: Cytat if ($row['dom_days_left']>1 && $row['dom_days_left']<=30) powinny być wyexportowane do innej metody. Np
Albo
Nie wiem, który wariant lepszy, ale ten drugi zdaje się nie pasować do klasy o nazwie Domain wtedy komentarz: Cytat // add domains between 1 and 30 days to result będzie niepotrzebny. Wadą tego rozwiązania jest oczywiście mniejsza wydajność (pewnie bardzo nieznaczna, ale w javie stosowany jest silny inlining, tutaj raczej nie bardzo.), większe rozbicie kodu (więcej metod) za to kod jest czytelniejszy i potrzebuje mniej komentarzy, które to lubią tracić na aktualności. Ten post edytował peter13135 13.06.2013, 21:33:28 -------------------- :)
|
|
|
13.06.2013, 21:37:15
Post
#13
|
|
Grupa: Moderatorzy Postów: 4 069 Pomógł: 497 Dołączył: 11.05.2007 Skąd: Warszawa |
|
|
|
13.06.2013, 21:43:34
Post
#14
|
|
Grupa: Zarejestrowani Postów: 1 447 Pomógł: 191 Dołączył: 26.03.2008 Ostrzeżenie: (0%) |
Fakt, Twoja lepsza Ale ideę chyba miałem w miarę dobrą.
Czy dasz mi jakieś "materiały" udowadniające słuszność zastosowania prefixu "_" w prywatnych składowych ? -------------------- :)
|
|
|
13.06.2013, 22:16:33
Post
#15
|
|
Grupa: Zarejestrowani Postów: 125 Pomógł: 9 Dołączył: 25.05.2013 Ostrzeżenie: (0%) |
progi 1 i 30 teraz są arbitralne ale będą definiowalne przez użytkownika nieco później (roadmap), więc zawczasu się przygotowałem do tego
ale można pomyśleć nad
-------------------- |
|
|
16.06.2013, 23:44:42
Post
#16
|
|
Grupa: Zarejestrowani Postów: 1 447 Pomógł: 191 Dołączył: 26.03.2008 Ostrzeżenie: (0%) |
Cytat @down - nie dziś Czyli pewnie ani jutro, ani po jutrze -------------------- :)
|
|
|
17.06.2013, 07:18:22
Post
#17
|
|
Grupa: Zarejestrowani Postów: 4 298 Pomógł: 447 Dołączył: 16.11.2006 Ostrzeżenie: (0%) |
Czy dasz mi jakieś "materiały" udowadniające słuszność zastosowania prefixu "_" w prywatnych składowych ? http://forum.php.pl/index.php?s=&showt...t&p=1041806 -------------------- Nie udzielam pomocy poprzez PW i nie mam GG.
Niektóre języki programowania, na przykład C# są znane z niezwykłej przenośności (kompatybilność ze wszystkimi wersjami Visty jest wiele warta). |
|
|
17.06.2013, 07:42:33
Post
#18
|
|
Grupa: Moderatorzy Postów: 4 069 Pomógł: 497 Dołączył: 11.05.2007 Skąd: Warszawa |
!*! - Było kiedyś coś ciekawszego.
|
|
|
17.06.2013, 09:13:46
Post
#19
|
|
Grupa: Zarejestrowani Postów: 1 447 Pomógł: 191 Dołączył: 26.03.2008 Ostrzeżenie: (0%) |
OK. przeczytałem. Nie mam zamiaru się kłócić, bo widzę, że zdania są podzielone. Do mnie jednak trafiają te argumenty :
1. Dobry kod nie posiada zmiennych publicznych. 2. IDE koloruje zmienne tak, że bez problemu można odróżnić, cy jest to zmienna lokalna czy instancji obiektu. -------------------- :)
|
|
|
Wersja Lo-Fi | Aktualny czas: 28.04.2024 - 15:27 |