Forum PHP.pl

Witam.

Nie twierdzę, że to co zaraz pokaże jest zgodne z całą "polityką" pisania obiektowego Pisze dla siebie i tak aby w miare mi było wygodnie "coś" rozwijać i takie tam. Oto taki proof on concept ;] Nawet nie sprawdzałem czy to działa, chodzi mi wyłącznie o ocene sposobu wykonania.

http://vengeance.strefaphp.net/news/source.php

więc czy takie coś jest dobre? jesli tak to co jeszcze zmienic/poprawic/usunac/dodac.

Jesli nie to jak to rozwiazac lepiej

Pozdrawiam,
Vengeance

Wiesz zdaje mi się że ta klasa jest do bani.Dlaczego? Bo w oop nie chodiz by na poczatku klasy deklrować wartosci wczytywane z bazy, chodzi o tok myslenia, czemu np. zamiast tego -> var $id, $author_id, $date, $image; nie zrobiłeś poprostu var $arrNews = array(); i już masz kod zoptymalizowany itp. raczej proponuje cie napisać od nowa.

bo klasa News reprezentuje jeden konkretny News.
I ma takie argumenty jakie posiada ten ów konkretny News.
Potem moge operować na tych argumentach dowolnie i jednym wywołaniem SaveData() zapisać zmiany do bazy.

Zaś NewsContainer reprezentuje zbiór wszystkich newsów (czyt. zbir obiektow klasy News)

Błędem jest umieszczenie w klasie News funkcji getData($id). Dlaczego? Bo można stworzyć sobie newsa, a potem przypisać mu inne dane. Nie wolno zmieniać znaczenia konkretnej instancji klasy, bo traci się tożsamość obiektu. Cieżko to wytłumaczyć bez użycia teorii, ale nie może być tak, że ten sam news raz jest jednym newsem, a za chwilę zupełnie innym.

Metoda getData() w klasie NewsContainer powinna zwracać iterator po newsach. Po co wczytywać wszystko na początku? Jest to nieefektywne i przede wszystkim b ardzo obciąża pamięć. Jeżeli piszesz pod PHP5, to iteratory masz wbudowane. Jeżeli pod PHP4, to i tak wszyscy wiedzą jak iterator powinien wyglądać.

Z usuwaniem newsa jest problem, bo co reprezentuje obiekt News po wywołaniu metody DeleteData() ? Tego newsa już przecież nie ma. Ale tego nie da się chyba elegancko rozwiązać.

Po co metoda SetSqlQuery() w NewsContainer? W ten sposób można wstawić tam coś zupełnie nie związanego z newsami.

No i na koniec najważniejsze: czy warto tworzyć klasy takie jak News i NewsContainer - typowe klasy mapujące bazę danych na OOP - jeżeli mamy do dyspozycji takie rzeczy jak DB_DataObject i Propel? To zależy, ale trzeba wiedzieć dlaczego się na coś decydujemy. Taka ogólna dygresja

@Kubatron: kłamiesz.
@Vengeance:

• Kod niezgodny ze standardami kodowania, więc lekko nieczytelny.
• Brak jakichkolwiek komentarzy
• W GetData() - nie mozna czasem wykonac tych rzypisań w jakiejśc pętli? Bo trochę długie to jest
• Generowanie zapytań Insert , Update, ... - patrz wyżej
• Dla jakiego php piszesz? 4 czy 5?

@hawk

Nom prawda. Ale jak np. rozwiązać zwracanie FALSE gdy dany news nie istnieje.


Hmm sam sobie odpowiedziales


Mozna / nie mozna. Tak samo moge powiedziec ze moge usunac jakies tam przypisania w skrypcie i tez sie wszystko posypie
A w zamierzeniu była po to aby tą jedną klasą wyciągać różne dane.
Raz np. o wszystkich newsach (dla panelu admina) drugi raz tylko aktywne (przy wyswietlaniu ich użytkownikowi) a trzeci raz tylko ostatnie 10 newsów. Jak inaczej mozna to rozwiązać ? Na zasadzie dziedziczenia z NewsContainer ?


Zaraz zapytam google o te "rzeczy"

@scanner

Myślałem potem nad automatyzacją tych procesów. jednak na razie w takiej formie będą poprawmy całą reszte


Dla 4

@ogolnie
Dzieki za komentarze Czlowiek uczy sie na wlasnych bledach i wlasnie zalezy mi na takim "opieprzaniu" wiedzac ze robi to ktos godny
Praktyka czyni mistrza....

A-ha! Bo News nie powinien tworzyć się sam z siebie. Powinien być tworzony przez kontener Newsów, który działa jak fabryka. I np. przyjmować id w konstruktorze. Robiłbyś $news = NewsContainer::getInstance()->getById(123456);


Dziedziczenie - absolutnie nie. Wtedy nagle 2 kontenery zaczną wczytywać sobie newsy z bazy danych, a przy cachowaniu będą robić 2x to samo.

Więc jak to zrobić?
1) Dodać metody wyciągające to co chcesz, np. getLatest($howMany), getActiveNews(), itd.
2) Dodać mechanizm definiowania (obiektowo) kryteriów, które można zapodawać takiemu kontenerowi i on na podstawie tego wygeneruje sobie odpowiedni SQL i wyciągnie co trzeba - tak robią to gotowe systemy które podałem.

A wstawianie na chama SQL jest złe, bo:
1) Po to robisz te klasy, żeby inne fragmenty kodu nie musiały zdawać sobie sprawy ze schematu bazy danych i nie zepsuły nic
2) Masz w zasadzie SqlQueryContainer, a nie NewsContainer, bo można tam wstawić SQL w ogóle nie związany z newsami, a klasa to przyjmie i zacznie robić straszne rzeczy, niezgodne ze swoim przeznaczeniem.

ok wielkie dzieki. Jeszcze kilka pytan w takim razie

[PHP] pobierz, plaintext 
<?php
 
   class News
   {
	  var $id, $author_id, $date, $image;
	  var $title, $short_text, $long_text;
	  var $source, $view, $active;
 
	  function getParam($key) { }
	  function setParam($key, $value) { }
}
 
?>
[PHP] pobierz, plaintext 

Czy zostawić klase News w takiej postaci (kod metod pominięto).
Chodzi mi oto czy w momencie pobrania newsa o danym ID
to kontener powinien pobierac dane ze SQL potem je "pakować" do obiektu News po czym go zwrócić ? Sądze, że tak jest lepiej a nie jak dotychczasz, że to klasa News sama w sobie pobiera informacje o newsie gdyż uniemożliwia to wpomnianą kontrolę błędów.

po 2.
Usuwanie newsa także powinno być w takim razie inicjowane przez kontener dzieki temu omijamy poprzedni problem " bo co reprezentuje obiekt News po wywołaniu metody DeleteData()".

po 3.
W jaki sposób "zorganizować" tworzenie nowego newsa. hmm...

cos kolo tego? :/

Wielkie dzięki za pomoc

nie powinno być tak ? :

[PHP] pobierz, plaintext 
<?php
$news = Kontener::instancja()->PobierzNowy();
$news->title=&#092;"aaa\";
$news->ZapiszZmiany();
?>
[PHP] pobierz, plaintext 

Ten post edytował bela_666 16.11.2004, 16:59:55

wlasnie chodzi mi oto jak powinno byc (dodawanie i zapisywanie zmian).

pozatym juz na pewno nie tak jak ty dales. ja widze 2 rozwiazania:
Kontener::instancja()->ZapiszZmiany($news);
i
$news->ZapiszZmiany();

choc 1 wydaje mi sie "lepsiejsza"