![]() |
![]() ![]() |
![]() |
![]()
Post
#1
|
|
Grupa: Zarejestrowani Postów: 13 Pomógł: 0 Dołączył: 14.08.2010 Ostrzeżenie: (0%) ![]() ![]() |
Witam, wiem że w tym dziale jest już świeży temat o stosowaniu OOP, jednak chciałbym aby ktoś zweryfikował mój kod czy to co robię jest zgodne z założeniem OOP.
Jest to przykładowy game framework, dla treningu. Mam w nim 4 klasy User, UserManager, Character, CharacterManager, oto one :
Index.php
Pytania: 1. Głowne - czy ten kod jest zgodny z OOP ? 2. Czy metoda Logon() powinna być w klasie UserManager czy w osobnej np. Login 3. Czy tworzenie obiektu User a potem przekazywanie go do UserManager w celu obróbki itp jest prawidłowe ? Czy w klasie UserManager powinna być metoda tworząca obiekt User ? (to tyczy się również klas Character i CharacterManager) Proszę o wytknięcie mi błedów i sposobów w jaki mogę je poprawić ![]() |
|
|
![]()
Post
#2
|
|
![]() Grupa: Zarejestrowani Postów: 130 Pomógł: 11 Dołączył: 7.04.2003 Ostrzeżenie: (10%) ![]() ![]() |
Cytat 1. Głowne - czy ten kod jest zgodny z OOP ? To czy jakiś obiekt ma sens, czyli czy powinien istnieć czy nie, nie wynika bezpośrednio z tego że program jest OOP tylko z dziedziny problemu który program rozwiązuje. Ale jeżeli hurtowo nazywasz swoje obiekty "manager" to coś może być nie tak. Zbyt ogólna nazwa sugeruje że obiekt robi zbyt wiele rzeczy lub że sam nie wiesz co ma robić. Cytat Czy metoda Logon() powinna być w klasie UserManager czy w osobnej np. Login Moja propozycja: Olej całkowicie logowanie. Skup się na problemie który twój program ma rozwiązywać. Cytat tworzenie obiektu User a potem przekazywanie go do UserManager w celu obróbki itp jest prawidłowe ? Czy w klasie UserManager powinna być metoda tworząca obiekt User ? (to tyczy się również klas Character i CharacterManager) Staramy się wstrzykiwać obiekty w inne obiekty a nie inicjalizować jedne w drugich. Oczywiście inicjalizacja musi gdzieś nastąpić. Także to że przekazujesz usera do innego obiektu jest ok, pytanie tyko czy wiesz do czego ma słuzyc manager? -------------------- .:SMENTEK:.
|
|
|
![]()
Post
#3
|
|
Grupa: Zarejestrowani Postów: 13 Pomógł: 0 Dołączył: 14.08.2010 Ostrzeżenie: (0%) ![]() ![]() |
No właśnie też złapałem się na tym, że rzucam nazwą manager jak popadnie, ale teraz wszędzie widzę do tego zastosowanie, nie wiem czemu :/ Mogę prosić o jakiś pomysł jak inaczej nazwać, rozbić te klasy ?
A co do tego czy wiem czemu ma służyć manager to chyba do tego aby zarządzać - dodawać, edytować, wybierać użytkowników itp. (w przypadku klasy UserManager). Cytat Olej całkowicie logowanie. Skup się na problemie który twój program ma rozwiązywać. Prawdę mówiąc nie wiem czy złapałem o co chodzi ![]() |
|
|
![]()
Post
#4
|
|
Grupa: Zarejestrowani Postów: 71 Pomógł: 0 Dołączył: 5.03.2011 Ostrzeżenie: (0%) ![]() ![]() |
mysle ze mozesz sobie poczytac moj post, zadawalem niemal ze identyczne pytania a dyskusja sie rozwinela calkiem na bogato.
Temat: prawidlowe OOP struktura klasy |
|
|
![]()
Post
#5
|
|
![]() Grupa: Zarejestrowani Postów: 175 Pomógł: 18 Dołączył: 11.06.2007 Skąd: Koszalin/Poznań Ostrzeżenie: (0%) ![]() ![]() |
Prawdopodobnie całkiem przez przypadek, ale zrobiłeś sobie małe Entity manager w stylu Doctrine 2. Zobacz jak tam jest rozwiązane zarządzanie obiektami które swoje właściwości zaciągają z bazy danych.
|
|
|
![]()
Post
#6
|
|
![]() Grupa: Moderatorzy Postów: 4 069 Pomógł: 497 Dołączył: 11.05.2007 Skąd: Warszawa ![]() |
Za łączenie z pdo powinna odpowiadać klasa db a nie userManager czy characterManager. Czemu 2 razy dajesz połączenie? Użyj dependency injection.
Echo i exception w modelu? Wywal echo. I tak jak pisze Rahul, zapoznaj się z podobnymi tematami. |
|
|
![]()
Post
#7
|
|
![]() Grupa: Zarejestrowani Postów: 130 Pomógł: 11 Dołączył: 7.04.2003 Ostrzeżenie: (10%) ![]() ![]() |
Cytat Prawdę mówiąc nie wiem czy złapałem o co chodzi ![]() Chodzi mi o to ze program mozna koncepcyjnie podzielić na: - Cześć APLIKACYJNA, która jest "wspólna" dla większości systemów www. Do tego wchodzi np. autoryzacja. (twoje logowanie) obsługa sesji, jakaś implementacja MVC (lub prostszy mechanizm zamiast MVC). Routingu. Tej części nie wymyślaj od nowa użyj jakiegoś frameworku. - Część BIZNESOWĄ która jest unikalna dla danego systemu. Jeżeli piszesz system do obsługi pizzerii to to będą obiekty Kucharz, Klient, Pizza, Składnik, Pizzy, Kelner, etc. (z palca pisze). Skup się na tej części biznesowej a pierwszą znajdziesz w gotowych rozwiązaniach. Obie warstwy powinny zajmować się różnymi rzeczami. To znaczy obiekt Kucharz nie powinien implementować logowania ani zapisu danych dobazy, nawet jeżeli koncepcyjnie użytkownik będący kucharzem może się logować. Obiekt Kucharz może za to przyjmować w konstruktorze obiekty Składaniki Pizzy oraz posiadać metodę makePizza() która zwraca Pizze. Poćwicz sobie tworzenie systemów składających się z małych obiektów które rozwiązują tego typu problemy. Bez tej umiejętności będziesz jak papuga która klepie bezmyślnie kod według koncepcji wymyślonych przez innych, których to koncepcji nawet nie rozumie, ale sam nie będziesz potrafił zaprojektować nawet prostego systemu... -------------------- .:SMENTEK:.
|
|
|
![]()
Post
#8
|
|
Grupa: Zarejestrowani Postów: 13 Pomógł: 0 Dołączył: 14.08.2010 Ostrzeżenie: (0%) ![]() ![]() |
Cytat mysle ze mozesz sobie poczytac moj post, zadawalem niemal ze identyczne pytania a dyskusja sie rozwinela calkiem na bogato. Temat: prawidlowe OOP struktura klasy Właśnie ten temat miałem na myśli pisząc, że jest tutaj świeży temat o OOP. Przestudiowałem twój temat parę razy, właśnie na jego podstawie napisałem to tutaj, jednak chciałbym aby ktoś spojrzał na mój kod. Cytat Prawdopodobnie całkiem przez przypadek, ale zrobiłeś sobie małe Entity manager w stylu Doctrine 2. Zobacz jak tam jest rozwiązane zarządzanie obiektami które swoje właściwości zaciągają z bazy danych. Poczytam o tym ![]() Cytat Za łączenie z pdo powinna odpowiadać klasa db a nie userManager czy characterManager. Czemu 2 razy dajesz połączenie? Użyj dependency injection. Echo i exception w modelu? Wywal echo. I tak jak pisze Rahul, zapoznaj się z podobnymi tematami. Ok, poprawię to. Zresztą miałem tak robić ale nie wiem czemu z tego zrezygnowałem ![]() Cytat Poćwicz sobie tworzenie systemów składających się z małych obiektów które rozwiązują tego typu problemy. Bez tej umiejętności będziesz jak papuga która klepie bezmyślnie kod według koncepcji wymyślonych przez innych, których to koncepcji nawet nie rozumie, ale sam nie będziesz potrafił zaprojektować nawet prostego systemu... Własnie wiem, że aktualnie jestem taką papugą ![]() Ogólnie wasze posty bardzo mi pomogły, dziękuję za zainteresowanie moim tematem mimo tego, że było już takich 2131545 ![]() Zakładając, że chcę napisać klasę do logowania/rejestracji, jak powinienem to rozdzielić ? Osobna klasa do rejestracji i logowania, czy np. ogólnie klasa która będzie odpowiadała za te czynności ? Bo w zasadzie taką klasą miała być klasa UserManager, w niej miały znajdować się rejestracja, logowanie, wylogowywanie, usuwanie userów itp. Czy jest to prawidłowe założenie ? @smentek Ta klasa byłaby w części aplikacyjnej tak ? A Character i CharacterManager to już część biznesowa tak? |
|
|
![]()
Post
#9
|
|
![]() Grupa: Zarejestrowani Postów: 1 366 Pomógł: 261 Dołączył: 23.09.2008 Skąd: Bydgoszcz Ostrzeżenie: (0%) ![]() ![]() |
jeżeli masz dane w tablicy o takich samych nazwach jak pola to możesz zrobić sobie metodę setFromArray zamiast tego:
Kod $this->_character->setCondition($character['condition']);
$this->_character->setCoordination($character['coordination']); $this->_character->setDynamic($character['dynamic']); $this->_character->setEndurance($character['endurance']); $this->_character->setExperince($character['experince']); $this->_character->setFlex($character['flex']); $this->_character->setIq($character['iq']); $this->_character->setLevel($character['level']); $this->_character->setLife($character['life']); $this->_character->setMoney($character['money']); $this->_character->setMovePoints($character['move_points']); $this->_character->setNeedExperince($character['needexperince']); $this->_character->setPain($character['pain']); $this->_character->setReputation($character['reputation']); $this->_character->setSpeed($character['speed']); $this->_character->setStrenght($character['strenght']); $this->_character->setTechnic($character['technic']); -------------------- |
|
|
![]()
Post
#10
|
|
![]() Grupa: Zarejestrowani Postów: 130 Pomógł: 11 Dołączył: 7.04.2003 Ostrzeżenie: (10%) ![]() ![]() |
Cytat Zakładając, że chcę napisać klasę do logowania/rejestracji, jak powinienem to rozdzielić ? Osobna klasa do rejestracji i logowania, czy np. ogólnie klasa która będzie odpowiadała za te czynności ? Im mniejsze czyli bardziej rozdrobnione obiekty tym lepszy system. Z drugiej strony jeżeli ma to być prosty system to nie ma sensu sztucznie go komplikować, tak że możesz rejestracje i logowanie zrobić w jednym obiekcie, tylko niech to nie będzie ten sam obiekt który reprezentuje użytkownika. Może 'UserAuthorisation'? Cytat Bo w zasadzie taką klasą miała być klasa UserManager, w niej miały znajdować się rejestracja, logowanie, wylogowywanie, usuwanie userów itp. Czy jest to prawidłowe założenie ? Tak ale nazwa tego obiektu nie mówi o tym co obiekt robi. Cytat @smentek Ta klasa byłaby w części aplikacyjnej tak ? A Character i CharacterManager to już część biznesowa tak? Charakter jest biznesowy. A CharacterManager to obiekt, który powinien zostać podzielony na tyle obiektów ile widzisz 'problemów' do rozwiązania z wiązanych z obiektem Character. Każdy nowy kontekst wprowadza nowe obiekty. Jeżeli chodzi o sam obiekt Character. Nawrzucałeś do niego różnych zmiennych które ci przyszły do głowy a które nie mają jakiegokolwiek uzasadnienia. Obiekt Character to nie jest prawdziwy obiekt w rozumieniu OOP. Od tablicy różni go tylko forma zapisu. Przykładowo $_speed. Czym jest speed? Prędkość z którą porusza się byt w danym momencie czy maksymalna prędkość z którą może się poruszać? Prędkość to jest coś względnego. Względem czego liczysz prędkość? Jeżeli nie masz tego czegoś to może i sama speed powinna na razie zniknąć? Może prędkość nie powinna być zmienną tylko wynikać z relacji obiektów, tak jak w świecie rzeczywistym? -------------------- .:SMENTEK:.
|
|
|
![]()
Post
#11
|
|
Grupa: Zarejestrowani Postów: 13 Pomógł: 0 Dołączył: 14.08.2010 Ostrzeżenie: (0%) ![]() ![]() |
Cytat Im mniejsze czyli bardziej rozdrobnione obiekty tym lepszy system. Z drugiej strony jeżeli ma to być prosty system to nie ma sensu sztucznie go komplikować, tak że możesz rejestracje i logowanie zrobić w jednym obiekcie, tylko niech to nie będzie ten sam obiekt który reprezentuje użytkownika. Może 'UserAuthorisation'? Brakowało mi zamiennika nazwy ![]() Cytat Jeżeli chodzi o sam obiekt Character. Nawrzucałeś do niego różnych zmiennych które ci przyszły do głowy a które nie mają jakiegokolwiek uzasadnienia. Obiekt Character to nie jest prawdziwy obiekt w rozumieniu OOP. Od tablicy różni go tylko forma zapisu. W którymś tematcie o OOP było napisane, że całkiem rozsądne jest robienie takich kontenerów-obiektów zawierających dane. Są tam zmienne, settery i gettery. Czy jednak to jest zło i należy to zmienić ? Cytat Przykładowo $_speed. Czym jest speed? Prędkość z którą porusza się byt w danym momencie czy maksymalna prędkość z którą może się poruszać? Prędkość to jest coś względnego. Względem czego liczysz prędkość? Jeżeli nie masz tego czegoś to może i sama speed powinna na razie zniknąć? Może prędkość nie powinna być zmienną tylko wynikać z relacji obiektów, tak jak w świecie rzeczywistym? Prawdę mówiąc w kontekście tego co chcę zrobić, nie potrzebuję takich "szerszych horyzontów" bo $_speed ma po prostu być zwykłą statystyką, na której podstawie będzie można wyliczyć np. szanse na unik/trafienie. To znowu ja ![]() Zmiany: Dodałem klasę UserAuthorisation dzięki której po przekezaniu obiektu User można się zalogować.
Dodałem klase Database do obsługi bazy danych, za wiele w niej nie ma ![]()
Zmiany w klasie UserManager, dodałem funkcję tworzącą użytkownika, wywalilem z niej funkcje logującą i dodałem wykorzystanie klasy Database.
W index.php dorzuciłem tworzenie usera.
Jest dobrze ? Ten post edytował Fanatyko 29.08.2011, 21:48:05 |
|
|
![]()
Post
#12
|
|
![]() Grupa: Zarejestrowani Postów: 135 Pomógł: 38 Dołączył: 24.02.2007 Skąd: Warszawa Ostrzeżenie: (10%) ![]() ![]() |
Dla mnie kod wygląda całkiem znośnie, ale:
1. Unikaj takiego kodu
Pokażesz wtedy jaki błąd wystąpił wszystkim użytkownikom (jak się łączysz, tabelę do której się łączysz), nie dość, że jest to nieeleganckie to i w przyszłości kiedyś ktoś może tą wiedzę wykorzystać przeciw Tobie (sql injection). Możesz sam napisać jakaś klasę do obsługi własnych błędów, a błędy systemowe zapisywać do jakich logów. 2.
Czepiam się, ale bardziej naturalne wydaje mi się, że funkcja getLogged powinna powinna być funkcją typu isLogged i zwracać wartość boolean. 3.UserManager.php
Nie podoba mi się zwracanie stringa, że funkcję create(). Czy zadaniem funkcji jest zwracaniem napisu? Raczej nie, według mnie nic nie powinna zwracać (najwyżej rzucać wyjątkiem podczas niepowodzenia) lub symbolicznie wartość logiczną w zależności od powodzenia. Jeśli bardzo chcesz już stosować jakieś stałe napisowe, to nie w ten sposób. Utwórz na przykład sobie statyczne pola z napisami, które będą symbolizowały statusy zakończony akcji i porównywuj tylko statusy, wtedy jeśli w przyszłości będziesz chciał zmienić napis to zrobisz to w jednym miejscu. Pozdrawiam -------------------- |
|
|
![]()
Post
#13
|
|
Grupa: Zarejestrowani Postów: 9 Pomógł: 1 Dołączył: 28.03.2011 Skąd: Bytom Ostrzeżenie: (0%) ![]() ![]() |
class Database { function __construct() { $this->_pdo = new PDO('mysql:host=localhost;dbname=rsfc;', 'root', 'beniamins9'); $this->_pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); } } wydaje mi się, że nie używa się polecenia echo w modelu bazy |
|
|
![]()
Post
#14
|
|
Grupa: Zarejestrowani Postów: 13 Pomógł: 0 Dołączył: 14.08.2010 Ostrzeżenie: (0%) ![]() ![]() |
@Fantazyn
dzięki za odpowiedź, naprawdę mi się przydała. Jakoś nie pomyślałem o logach, nie wiem czemu ![]() ![]() @banditpanda poprawię to ![]() Ten post edytował Fanatyko 29.08.2011, 21:54:37 |
|
|
![]()
Post
#15
|
|
![]() Grupa: Zarejestrowani Postów: 130 Pomógł: 11 Dołączył: 7.04.2003 Ostrzeżenie: (10%) ![]() ![]() |
Cytat "W którymś tematcie o OOP było napisane, że całkiem rozsądne jest robienie takich kontenerów-obiektów zawierających dane. Są tam zmienne, settery i gettery. Czy jednak to jest zło i należy to zmienić ?" Ale taki obiekt który ma tylko dane + gettery i settery jest jak lodówka która nie mrozi albo auto które nie jeździ. Niewiele różni się od tablicy albo zbioru luźnych zmiennych. Zależy czym ma być ten obiekt.Tu są dwie strony medalu: 1. Getery i setery są konwencją która pozwala na pewnego rodzaju "rozpoznawalny" interfejs dostępu do danych obiektu. Dzięki czemu łatwiej zaprzęgnąć do pracy zewnętrzną bibliotekę tak aby współdziałała z twoimi obiektami. Np chcesz wprowadzić zapis obiektów do bazy danych. Zamiast pisać własny kod który to obsługuje korzystasz z Doctrine2. Albo chcesz przeprowadzać inicjalizację przez kontener IoC. Jakikolwiek nie był by to mechanizm musi jakoś obsługiwać twoje obiekty, zrobi to dzięki getterom i setterom które umieścisz w swoim obiekcie. (To nie musi być zewnętrzny system to moze byc system który sam chcesz napisac) I to jest dobre prawidłowe wykorzystanie get i set. 2. Getery i setery mogą też stać się 'protezami' dla programisty który nie potrafi zaplanować dobrze kodu i zamiast prosić obiekt aby ten coś dla niego wykonał co chwile używa get i set aby cos z obiektu wyciągnąć albo cos do niego wrzucic. Czyli zamiast
piszesz
I to jest złe stosowanie get i set. Jezeli wprowadzasz get i set do swojego systemu nie wiedząc DOKLADNIE do czego jest ci to potrzebne to już zrobiłeś błąd. Ten post edytował smentek 30.08.2011, 20:31:53 -------------------- .:SMENTEK:.
|
|
|
![]()
Post
#16
|
|
Grupa: Zarejestrowani Postów: 13 Pomógł: 0 Dołączył: 14.08.2010 Ostrzeżenie: (0%) ![]() ![]() |
Cytat I to jest złe stosowanie get i set. Jezeli wprowadzasz get i set do swojego systemu nie wiedząc DOKLADNIE do czego jest ci to potrzebne to już zrobiłeś błąd. Czyli tak, zrobiłem błąd, wrzuciłem settery i gettery bo wszędzie o tym trąbią i że powinno się je mieć. A więc np. jeśli mam taki kod :
to powinienem to zrobić tak:
i wtedy konstruktorze wykorzystać settery czy normalnie przypisać do zmiennych ? Cytat Ale taki obiekt który ma tylko dane + gettery i settery jest jak lodówka która nie mrozi albo auto które nie jeździ. Niewiele różni się od tablicy albo zbioru luźnych zmiennych. Bez tego obiektu nie mam czego wstrzyknać do klas UserManager, UserAuthorisation. Coś muszę im przekazywać, przynajmniej tak mi się zdaję. Ten post edytował Fanatyko 30.08.2011, 22:55:29 |
|
|
![]()
Post
#17
|
|
Grupa: Zarejestrowani Postów: 10 Pomógł: 0 Dołączył: 3.09.2011 Ostrzeżenie: (0%) ![]() ![]() |
Główny problem tej klasy na co już niektórzy zwrócili uwagę to tworzenie nowego połączenia za każdym razem gdy tworzony jest nowy obiekt .
-------------------- Jestem za a nawet przeciw - http://forum.php.pl
|
|
|
![]()
Post
#18
|
|
![]() Grupa: Zarejestrowani Postów: 2 958 Pomógł: 574 Dołączył: 23.09.2008 Skąd: wiesz, że tu jestem? Ostrzeżenie: (0%) ![]() ![]() |
Mi nie podoba się jedna rzecz... zrobiłeś z klasy User kontener dla danych. Po co klasie User wiedzieć jaki login a tym bardziej hasło jest przekazywane przez formularz? Od tego masz klasę UserAuthorisation, która to może (ale nie musi) wypełnić danymi klasę User wysyłając do niej unikalne id użytkownika. Dobry przykład autoryzacji znajdziesz w książce PHP5 Zaawansowane programowanie
|
|
|
![]() ![]() |
![]() |
Aktualny czas: 20.08.2025 - 20:17 |