[Kohana]Kontrola jakości kodu |
[Kohana]Kontrola jakości kodu |
26.02.2013, 00:45:03
Post
#1
|
|
Grupa: Zarejestrowani Postów: 1 429 Pomógł: 195 Dołączył: 6.10.2008 Skąd: Kraków/Tomaszów Lubelski Ostrzeżenie: (0%) |
Witam. Skończyłem mały projekt w Kohanie (prosta strona z newsami, galerią itp.). Nie ma sensu pokazywać strony bo najważniejsza jest w tym właśnie Kohana i szlifowanie jej. Dopiero zacząłem się go uczyć i chciałbym przejść swego rodzaju kontrolę jakości. Wiadomo, prawdopodobnie "forumowy sanepid" zamknąłby moją fabrykę kodu ale głównie chodzi mi o uzyskanie rad, co robię źle i jak to poprawić żeby było dobrze (albo chociaż lepiej). Przymierzam się w niedalekiej przyszłości do stworzenia większego projektu i chciałbym żeby to było zrobione PRO.
Chciałbym abyście przyjrzeli się kontrolerowi i modelowi newsów (dla panelu administracyjnego). Proszę nie traktować tego jako "kodu do oceny". Ja wiem że to do oceny się jeszcze nie nadaje, z tym że ja już nie bardzo wiem co mógłbym poprawić. Stąd moja prośba o "kontrolę" kodu. Wybaczcie durnowate komentarze w kodzie, czasami się gubiłem. Kontroler: http://wklej.org/id/966403/ Model: http://wklej.org/id/966404/ Pozdrawiam Ten post edytował MateuszS 26.02.2013, 01:07:52 -------------------- O! Zimniok :P
|
|
|
26.02.2013, 01:16:27
Post
#2
|
|
Grupa: Zarejestrowani Postów: 2 355 Pomógł: 533 Dołączył: 15.01.2010 Skąd: Bydgoszcz Ostrzeżenie: (0%) |
Ciągłe odwoływanie się do funkcji statycznych, jest cechą tego frameworka? Czy Ty tak piszesz po prostu?
Ten post edytował Damonsson 26.02.2013, 08:40:59 |
|
|
26.02.2013, 08:08:35
Post
#3
|
|
Grupa: Zarejestrowani Postów: 395 Pomógł: 80 Dołączył: 24.08.2009 Ostrzeżenie: (0%) |
Parę uwag z którymi nie wszyscy się zgodzą:
Kod jest rozwlekły, to znaczy twój podany przez Ciebie kod nie potrzebuje tylu komentarzy, tak wiem że często wymagane jest na uczelaniach aby komentować wszystko co się da ale komentarze odwracają uwagę od kodu właściwego. Tak naprawdę gdyby usunąć je wszystkie to i tak kod jest czytelny. Jedynie warto zachować komentarze nad metodami. W przypadku "if-ów" i "else-ów" zawierających po jednej instrukcji powinieneś skorzystać z operatora trójargumentowego. Kod kontrolera nie powinien przekraczać 30 linijek, choć oczywiście nie jest to reguła a raczej zalecenie. Uwagi dotyczą kodu przedstawionego, w przypadku rozwiniętego modelu kod musi mieć więcej komentarzy, ponieważ są to sprawy indywidualne a nie zwykły CRUD. -------------------- |
|
|
26.02.2013, 08:10:11
Post
#4
|
|
Grupa: Zarejestrowani Postów: 4 340 Pomógł: 542 Dołączył: 15.01.2006 Skąd: Olsztyn/Warszawa Ostrzeżenie: (0%) |
Troche za dużo kodu w kontrolerze
Validacja i dodawanie rekorów to w modelu się odbywa. Moje metody odpowiedzialne za dodawanie i edycje rekordu wyglądają tak
Dodatkowo przenieś sobie metode message do kontrollera po którym dziedziczysz żebyś w każdym nie musiał sobie czegoś takiego dopisywać. A model jak model nic szczególnego, tylko dlaczego wszystko na bool rzutujesz nie jest Ci potrzebne ID rekordu ktory dodałeś ? Liczba usuniętych / zmienionych rekordów
-------------------- I'm so fast that last night I turned off the light switch in my hotel room and was in bed before the room was dark - Muhammad Ali.
Peg jeżeli chcesz uprawiać sex to dzieci muszą wyjść, a jeżeli chcesz żeby był dobry ty też musisz wyjść - Al Bundy. QueryBuilder, Mootools.net, bbcradio1::MistaJam http://www.phpbench.com/ |
|
|
26.02.2013, 13:25:05
Post
#5
|
|
Grupa: Zarejestrowani Postów: 872 Pomógł: 94 Dołączył: 31.03.2010 Ostrzeżenie: (0%) |
@Damonsson:
gdzie wedlug Ciebie ciagle odwoluje sie do metod statycznych poza implementacja wzorca fabryki(Class::factory()), ktory jakby nie bylo polega na tworzeniu metod statycznych zwracajacych instancje danej klasy? Ogolnie jest dobrze, gdyby kazdy pisal przynajmniej na takim poziomie swiat bylby lepszy Pare uwag odemnie: 1. Model_News: news_get_all() (i inne metody pobierajace dane) - po co zwracasz tablice (as_array()), IMO zwracanie iteratora jest lepszym rozwiazaniem m. in. z uwagi na dostep do roznych metod tego iteratora i mozliwosc ich rozszerzania. news_exists($id) - gdybys uzywal wyzej wspomnianych iteratorow moglbys zrobic tak:
2. Obiekt walidacji moglbys zwracac w modelu, zamiast tworzyc go w kontrolerze. 3. Nie uzywasz transakcji na bazie danych 4. Moze warto w Model_News zaimplementowac wzorzec fabryki (Model_News::factory())? Korzystasz z przefiltrowanych danych (pobierasz je z obiektu walidatora), co sie bardzo chwali. |
|
|
26.02.2013, 14:01:45
Post
#6
|
|
Grupa: Zarejestrowani Postów: 175 Pomógł: 26 Dołączył: 13.09.2007 Skąd: Gdańsk Ostrzeżenie: (0%) |
Z czysto konwencyjnych rzeczy, to z tego co pamiętam, Kohana nie lubi takich konstrukcji:
poza użyciem continue i return zaleca się zawsze stosowanie klamer { } -> http://kohanaframework.org/3.3/guide/kohana/conventions Jeśli używasz kohany 3.3, to nigdy nie używaj gołego $_POST, tak jak tu:
W przypadku takiej konstrukcji:
Nie używaj set, tylko bind. Jeśli do zmiennej zostanie coś wpisanego później w kodzie 'na dole', to zostanie to 'podłączone' dzięki bind 'na górze'. Ten post edytował sabat24 26.02.2013, 14:10:03 |
|
|
26.02.2013, 15:21:54
Post
#7
|
|
Grupa: Zarejestrowani Postów: 673 Pomógł: 106 Dołączył: 31.12.2008 Ostrzeżenie: (0%) |
nie piszę w kohanie, ale z mojej strony powiem że preferuję zapis (zmienne, metody itp. cuda - zmyślone):
zamiast:
musisz mieć tylko wtedy pewność np. jeśli zamiast return false masz przekierowanie/widok etc. że ta czynność blokuje wykonywanie dalszego kodu PS pewnie ma tylu zwolenników, co i przeciwników? -------------------- |
|
|
26.02.2013, 15:32:39
Post
#8
|
|
Grupa: Zarejestrowani Postów: 4 340 Pomógł: 542 Dołączył: 15.01.2006 Skąd: Olsztyn/Warszawa Ostrzeżenie: (0%) |
@r4xz kohana jaki i inne FW narzucają pewną stylistyke kodu, więc niech autor trzyma się tego co napisano
http://kohanaframework.org/3.3/guide/kohana/conventions -------------------- I'm so fast that last night I turned off the light switch in my hotel room and was in bed before the room was dark - Muhammad Ali.
Peg jeżeli chcesz uprawiać sex to dzieci muszą wyjść, a jeżeli chcesz żeby był dobry ty też musisz wyjść - Al Bundy. QueryBuilder, Mootools.net, bbcradio1::MistaJam http://www.phpbench.com/ |
|
|
26.02.2013, 16:02:43
Post
#9
|
|
Grupa: Zarejestrowani Postów: 1 429 Pomógł: 195 Dołączył: 6.10.2008 Skąd: Kraków/Tomaszów Lubelski Ostrzeżenie: (0%) |
Dokładnie. Na szczęście konwencji się jest łatwo nauczyć i faktycznie za późno zacząłem się tym interesować i mój kod był daleki od standardów Kohany.
Zainteresowało mnie to co napisał @skowron-line jeśli chodzi o ilość kodu w kontrolerze. Mianowicie spotkałem się (m. in. na forum polskim Kohany) ze stwierdzeniem że w modelu mamy tylko współpracę z bazą/obróbkę danych a walidacja odbywa się w kontrolerze. Mógłbyś pokazać jak u Ciebie wyglądałby model jeśliby przenieść tam walidację? Bo ten model przecież musi zwrócić jakieś błędy do kontrolera który to dopiero przekazuje je do widoku zgodnie z Tak więc wolałem sobie oszczędzić komplikacji z przekazywaniem błędów/komunikatów/wartości z modelu do kontrolera i zostawiłem same wartości. Ale jestem ciekaw jak Wy to rozwiązaliście? Co do komentarzy, tak jak pisałem, wiem że za dużo itp. ale już nie chciało mi się ich kasować przed wysłaniem, będę miał to na uwadze aby nie było ich za dużo. Cytat 3. Nie uzywasz transakcji na bazie danych Chodzi o jakieś współbieżne aktualizacje pól w MySQL? Ten post edytował MateuszS 26.02.2013, 16:09:59 -------------------- O! Zimniok :P
|
|
|
26.02.2013, 16:30:42
Post
#10
|
|
Grupa: Moderatorzy Postów: 4 069 Pomógł: 497 Dołączył: 11.05.2007 Skąd: Warszawa |
Za brak klamerek powinno się łamać klawiatury.
@MateuszS - twój obrazek nie pokazuje mvc tylko mvp. Co do pisania kontrolerów to zawsze były osoby piszące gruby kontroler, chudy model i odwrotnie. Zaraz pewnie powstanie flame na poziomie czy camel case jest lepsze. |
|
|
26.02.2013, 16:49:23
Post
#11
|
|
Grupa: Zarejestrowani Postów: 4 340 Pomógł: 542 Dołączył: 15.01.2006 Skąd: Olsztyn/Warszawa Ostrzeżenie: (0%) |
Chudy kontroller i gruby model - taka filozofie wyznaje
W kontrolerze tworząc instancje klasy przekazuje do modelu dane z post i get
i w modelu w metodzie save
a w kontrolerze mam tak
-------------------- I'm so fast that last night I turned off the light switch in my hotel room and was in bed before the room was dark - Muhammad Ali.
Peg jeżeli chcesz uprawiać sex to dzieci muszą wyjść, a jeżeli chcesz żeby był dobry ty też musisz wyjść - Al Bundy. QueryBuilder, Mootools.net, bbcradio1::MistaJam http://www.phpbench.com/ |
|
|
26.02.2013, 23:25:22
Post
#12
|
|
Grupa: Zarejestrowani Postów: 1 429 Pomógł: 195 Dołączył: 6.10.2008 Skąd: Kraków/Tomaszów Lubelski Ostrzeżenie: (0%) |
Po prostu przypisujesz tablicę do tablicy a potem metodą error() ją zwracasz tak? @Spawnm, szybko znaleziony na Google więc możliwe. Ale układ jest ten sam co w MVC. Kontroler ma "dostęp" do widoku, model już nie. PS. Czyli to czy umieszczę w kontrolerze walidację czy w modelu to kwestia obranej drogi tak? Nie jest błędem pisanie "grubego kontrolera"? Ten post edytował MateuszS 26.02.2013, 23:26:42 -------------------- O! Zimniok :P
|
|
|
27.02.2013, 01:39:46
Post
#13
|
|
Grupa: Zarejestrowani Postów: 283 Pomógł: 34 Dołączył: 21.03.2008 Ostrzeżenie: (0%) |
Wszystko trzeba robić z umiarem, również za słuszniejsze uważam umieszczanie walidacji w modelu.
Co do kodu, przede wszystkim zauważ, że wykonujesz te same lub podobne czynności dodając i edytując - większość mogłaby być napisana tylko raz. Prawdopodobnie nie potrzebujesz też osobnych widoków - przeważnie formularz edycji i dodawania wygląda tak samo. W action_edit i action_add zostawiłbym część odpowiedzialną za wyświetlanie widoku, natomiast zapis mógłby spocząć w odosobnieniu. Czyli dwie akcje, jeden widok, jeden zapis. -------------------- Free Web Tools - narzędzia dla programistów, webdeveloperów i specjalistów seo...
|
|
|
27.02.2013, 08:05:59
Post
#14
|
|
Grupa: Zarejestrowani Postów: 4 340 Pomógł: 542 Dołączył: 15.01.2006 Skąd: Olsztyn/Warszawa Ostrzeżenie: (0%) |
Po prostu przypisujesz tablicę do tablicy a potem metodą error() ją zwracasz tak? Tak PS. Czyli to czy umieszczę w kontrolerze walidację czy w modelu to kwestia obranej drogi tak? Nie jest błędem pisanie "grubego kontrolera"? I tak i nie. Wiele osób napisze Ci ze za przetwarzanie danych odpowiedzialny jest model, a kontroler łączy model z widokiem.
i to koniec, jeżeli chesz spawdzić i wyświetlić błąd związany z brakiem danych do wyświetlenia to możesz to zrobić w widoku, ale też w mojej opini model też by był dobrym miejscem na sprawdzenie, ale wszystko zależy od tego jak masz zaprojektowaną aplikację. -------------------- I'm so fast that last night I turned off the light switch in my hotel room and was in bed before the room was dark - Muhammad Ali.
Peg jeżeli chcesz uprawiać sex to dzieci muszą wyjść, a jeżeli chcesz żeby był dobry ty też musisz wyjść - Al Bundy. QueryBuilder, Mootools.net, bbcradio1::MistaJam http://www.phpbench.com/ |
|
|
27.02.2013, 17:52:14
Post
#15
|
|
Grupa: Zarejestrowani Postów: 1 429 Pomógł: 195 Dołączył: 6.10.2008 Skąd: Kraków/Tomaszów Lubelski Ostrzeżenie: (0%) |
Dzięki wszystkim za rady, postaram się ten kod przerobić tak jak zaleciliście (przynajmniej ten od newsów bo całego już nie dam rady, będę wiedział na przyszłość jak pisać)
-------------------- O! Zimniok :P
|
|
|
Wersja Lo-Fi | Aktualny czas: 16.04.2024 - 19:47 |