Początki z programowaniem obiektowym - klasa artykuł, Proszę o opinie sugestie |
Początki z programowaniem obiektowym - klasa artykuł, Proszę o opinie sugestie |
24.08.2009, 23:50:13
Post
#1
|
|
Grupa: Zarejestrowani Postów: 72 Pomógł: 0 Dołączył: 15.09.2008 Ostrzeżenie: (0%) |
Witam
Od jakiegoś czasu interesuje się programowaniem obiektowym, ale od niedawna zdecydowałem się stworzyć coś własnego. Będzie to mój pierwszy obiektowy (przynajmniej w części) CMS. chciałbym przedstawić wam klasę artykułu do oceny. Prosze o konstruktywną krytykę
Ten post edytował erix 25.08.2009, 11:26:21
Powód edycji: [erix] przeniosłem
|
|
|
25.08.2009, 08:59:16
Post
#2
|
|
Grupa: Zarejestrowani Postów: 419 Pomógł: 42 Dołączył: 12.08.2008 Skąd: Wrocław Ostrzeżenie: (0%) |
Bezsensowna sprawa jak dla mnie. Po pierwsze gdzie tu przemyślenie i trzymanie się jednego sposobu pisania metod ? Skoro getOne w ładny sposób czyta czy istnieje zmienna którą ktoś podał w parametrze i ją zwraca to w getAll powinna pętle przechodzić po wszystkich zmiennych klasowych i je dodawać do tablicy zamiast ręcznie rozpisujesz - podstawa to konsekwencja. Po drugie co to jest za klasa ? to kontroler, model, jakieś pudełko czy co ? Rzucanie wyjątkami na lewo i prawo jest trochę dziwne w tym wypadku. Moim zdaniem to powinno służyć tylko do wywalania krytycznych błędów lub czegoś nie do obsłużenia. W przypadku braku zmiennej coś takiego wyrzucać ? Poza tym klasa ma możliwość dodania artykułu pod warunkiem, że przy instancjonowaniu podamy produkt już ISTNIEJĄCY. A jak baza jest pusta to nie dodamy produktu bo się nam obiekt nie utworzy ? Można by się jeszcze poczepiać tylko brakuje mi czasu.
|
|
|
25.08.2009, 09:08:31
Post
#3
|
|
Grupa: Zarejestrowani Postów: 72 Pomógł: 0 Dołączył: 15.09.2008 Ostrzeżenie: (0%) |
Cytat Poza tym klasa ma możliwość dodania artykułu pod warunkiem, że przy instancjonowaniu podamy produkt już ISTNIEJĄCY. A jak baza jest pusta to nie dodamy produktu bo się nam obiekt nie utworzy ? Mozna dodawać także i nowe jeszcze nie zapisane do bazy artykuły. Do tego służy metoda CreateArt, która wykona się tylko jeśli wartość pola "new" jest równa true. Jest ono ustawiane własnie podczas tworzenia obiektu w konstruktorze. Jeśli podamy w nim id to obiekt bedzie utworzony na podstawie danych z bazy, a jeśli nie to bedzioe oznaczone własnie jako nowy. Co do nadmiernych wyjątków, myslisz, że lepiej będzie zwracać po prostu false i dopiero przy uzywaniu takiego obiektu i zwróceniu false odpowiednio wyświetlić komunikat lub wyrzucić wyjątek? Ten post edytował czarek1986 25.08.2009, 09:10:36 |
|
|
25.08.2009, 10:01:49
Post
#4
|
|
Grupa: Moderatorzy Postów: 4 362 Pomógł: 714 Dołączył: 12.02.2009 Skąd: Jak się położę tak leżę :D |
A po co dodatkowy parametr new? ID w zupełności go zastępuje W końcu jeśli jest tam NULL, to spełnia on tę samą rolę. Sprawdzasz więc jedynie jaką wartość ma ID i w razie czego podejmujesz decyzję czy robić UPDATE czy INSERT w bazie. To sprawia, że createArt i updateArt możesz zastąpić jedną metodą. Konstruktor w zasadzie działałby podobnie. Brak id -> nowy z wartościami domyślnymi, prawidłowe id -> pobiera artykuł, nieprawidłowe id -> nowy + error. Kwestia funkcji do wyświetlania wszystkiego jest tak naprawdę sporna. Można użyć funkcji getOne, ale używanie funkcji w funkcji to zawsze minimalny narzut czasowy przy wykonywaniu. Więc patrząc od strony wydajności odwołanie się prosto do obiektu pozwala na ominięcie tworzenia zmiennych tymczasowych, odwołania do funkcji, powrotu do funkcji macierzystej, niszczenie zmiennych tymczasowych na stosie itp. Nie jest więc takie głupie jak się może wydawać bo oszczędza nam zarówno czas jak i pamięć serwera. Poza tym popatrz na getOne... Wywołujesz w niej za każdym razem funkcje i warunki. To spowalniałoby getAll za każdym razem.
Zastanawiałbym się także nad kategoria i tagi. Wszystko zależnie od tego jak masz zamiar rozwiązać to od strony bazy. Do przemyślenia jest bowiem choćby to, czy artykuł może być dodany do jednej, czy wielu kategorii. Bo jeśli do wielu to musisz nieco inaczej zapytania popisać. To samo tyczyło by ewentualnego dodania do klasy zmiennej dotyczącej tagów. -------------------- Najpierw był manual... Jeśli tam nie zawarto słów mądrości to zapytaj wszechwiedzącego Google zadając mu własciwe pytania. A jeśli i on milczy to Twój problem nie istnieje :D
|
|
|
25.08.2009, 10:17:22
Post
#5
|
|
Grupa: Zarejestrowani Postów: 72 Pomógł: 0 Dołączył: 15.09.2008 Ostrzeżenie: (0%) |
Czyli widzę ze zdania podzielone sa co do metody pobierającej wszystkie pola obiektu Na razie zostawie ją tak jak jest bo bardziej przemawia do mnie argument, że jest ona szybsza niż jakby miałą wykonywać jeszcze szereg dodatkowych funckji .
Co do tej zmiennej new to rzeczywiscie mozna to wywalić i zrobić sprawdzanie za pomocą id. Tak zrobie i dzieki za radę. A jak z tymi wyjątkami? Nie powinienem umieszczać ich wewnątrz klasy? |
|
|
25.08.2009, 10:38:15
Post
#6
|
|
Grupa: Moderatorzy Postów: 4 362 Pomógł: 714 Dołączył: 12.02.2009 Skąd: Jak się położę tak leżę :D |
Co do wyjątków zdania są podzielone. Puryści uważają, że zostały one stworzone do obsługi błędów i tylko tak je trzeba stosować. Inni używają jak leci, w tym do przekazywania informacji na stronie. Jest to więc tak naprawdę osobista preferencja. Ja osobiście bliższy jestem pierwszej grupie. Informacja o błędach powinna jak najmniej mówić userowi o strukturze serwisu, jego bazach itp, bo to ułatwia ewentualny atak. I dlatego obsługa błędów powinna być używana tylko w miejscach wrażliwych na wystąpienie błędu, by nie dopuścić do sytuacji jaką opisałem w zdaniu poprzednim. W kodzie, który jest pewny można sobie wyjątki darować, bo to w końcu dodatkowa funkcja sprawdzająca efekty wykonywania skryptu i w minimalny sposób go spowalnia.
Ten post edytował thek 25.08.2009, 10:39:23 -------------------- Najpierw był manual... Jeśli tam nie zawarto słów mądrości to zapytaj wszechwiedzącego Google zadając mu własciwe pytania. A jeśli i on milczy to Twój problem nie istnieje :D
|
|
|
25.08.2009, 20:49:55
Post
#7
|
|
Grupa: Zarejestrowani Postów: 72 Pomógł: 0 Dołączył: 15.09.2008 Ostrzeżenie: (0%) |
Troszeczkę przerobniłem moją klasę, według porad oraz własnych pomysłow. Teraz wyszło mi coś takiego.
Dodałem pole dbhandle w którym przechowywany będzie obiekt bazy danych. Kolejne dodane pole to $fieldsRequired w którym przechowywana jest tablica z polami wymaganymi do uzupełnienia. Zmodyfikowałem tez funkcję, która ma sprawdzać czy pola te są wypełnione. Na koniec połączyłem metody createArt i UpdateArt w jedną metodę SaveArt i usunełem pole new, a zamiast niego dałem sprawdzanie po id, według wczęsniejszej porady. Jak teraz według was wyglada moja klasa? |
|
|
26.08.2009, 00:15:35
Post
#8
|
|
Grupa: Zarejestrowani Postów: 651 Pomógł: 28 Dołączył: 4.12.2004 Ostrzeżenie: (0%) |
Bieżący piszemy przez ż z kropką.
Jak komentujesz metody i zmienne, to rób to konsekwentnie i komentuj wszystkie. Zachowaj jakiś standard komentowania - np. phpDocumentator. -------------------- Sygnatura niezgodna z regulaminem.
|
|
|
26.08.2009, 07:48:00
Post
#9
|
|
Grupa: Zarejestrowani Postów: 662 Pomógł: 45 Dołączył: 26.03.2007 Skąd: Warszawa Ostrzeżenie: (0%) |
Poczytaj o metodach magicznych.
|
|
|
26.08.2009, 09:15:09
Post
#10
|
|
Grupa: Zarejestrowani Postów: 72 Pomógł: 0 Dołączył: 15.09.2008 Ostrzeżenie: (0%) |
Bieżący piszemy przez ż z kropką. Jak komentujesz metody i zmienne, to rób to konsekwentnie i komentuj wszystkie. Zachowaj jakiś standard komentowania - np. phpDocumentator. Komentarze będą oczywiście przy każdej metodzie, na razie ich nie dodawałem bo nie wiem czy zaraz nie bedzie trzeba i tak tego usuwac |
|
|
26.08.2009, 09:58:50
Post
#11
|
|
Grupa: Zarejestrowani Postów: 300 Pomógł: 32 Dołączył: 31.07.2006 Ostrzeżenie: (0%) |
|
|
|
Wersja Lo-Fi | Aktualny czas: 24.04.2024 - 14:14 |