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 :

[PHP] pobierz, plaintext 
<?php class User {
 
    function __construct() {
 
    }
 
    private $_login = null;
    private $_password = null;
    private $_email = null;
    private $_id = null;
    private $_logged = null;
 
 //  tutaj dalej sa standardowe settery i gettery
 
} ?>
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
<?php
class UserManager {
 
    function __construct($user) {
        $this->_user = $user;
 
        try{
        $this->_pdo = new PDO('mysql:host=localhost;dbname=rsfc;', '', '');
        $this->_pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        }catch(PDOException $error){
            echo 'Problem z baza :( :' . $error->getMessage();
        }
    }
 
    private $_user;
 
    public function Logon(){
 
        $db = $this->_pdo->prepare('SELECT * FROM user_account WHERE login = :login AND password = :password');
        $db->execute(array(':login' => $this->_user->getLogin(), ':password' => $this->_user->getPassword()));
 
        try{
 
             if($user = $db->fetch()){
                 echo "Zostales zalogowany";
                 $this->_user->setId($user[id]);
                 $this->_user->setLogged(1);
                 $this->_user->setSession();
             }
 
                else{
                        throw new Exception("Nie ma Cie w bazie ! Nie zostales zalogowany :(");
                    }
 
        }catch(Exception $error){
            echo $error->getMessage();
        }      
 
    }
 
 
}
 
?>
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
<?php
class Character {
 
    function __construct() {
 
    }
 
    private $_strenght;
    private $_speed;
    private $_dynamic;
    private $_condition;
    private $_pain;
    private $_endurance;
    private $_iq;
    private $_coordination;
    private $_technic;
    private $_flex;
    private $_life;
    private $_level;
    private $_experince;
    private $_needexperince;
    private $_money;
    private $_reputation;
    private $_movepoints;
 
    private $_id = null;
 
    //settery
 
    // tutaj tez standardowe settey i gettery
 
 
}
 
?>
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
<?php
class CharacterManager {
 
    function __construct($character) {
        $this->_character = $character;
 
        try{
        $this->_pdo = new PDO('mysql:host=localhost;dbname=rsfc;', '', '');
        $this->_pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        }catch(PDOException $error){
            echo 'Problem z baza :( :' . $error->getMessage();
        }
 
 
    }
 
 
 
    private $_character;
    private $_pdo;
 
    public function render(){
        include './views/character.php';
    }
 
    public function upgradeStatistic($statistic, $how_much){
 
 
            $db = $this->_pdo->prepare('UPDATE user_character SET '.$statistic.' = '.$statistic.' + :how_much WHERE id = :id');
            $db->execute(array(':how_much' => $how_much, ':id' => $this->_character->getIdFromSession()));
 
    }
 
    public function downgradeStatistic($statistic, $how_much){
 
 
            $db = $this->_pdo->prepare('UPDATE user_character SET '.$statistic.' = '.$statistic.' - :how_much WHERE id = :id');
            $db->execute(array(':how_much' => $how_much, ':id' => $this->_character->getIdFromSession()));
 
    }
 
    public function setStatistics(){
        try{
 
            $db = $this->_pdo->prepare('SELECT * FROM user_character WHERE id = :id');
            $db->execute(array(':id' => $this->_character->getIdFromSession()));
 
            if($character = $db->fetch()){
                $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']);
 
            }else{
                throw new Exception('Nie moge uaktualnic statystyk :(');
            }         
            }
            catch(Exception $error){
                echo 'Wystapil blad:' . $error->getMessage();
            }
        }
 
}
?>
[PHP] pobierz, plaintext 

Index.php

[PHP] pobierz, plaintext 
<?php
session_start();
 
require 'class/Character.php';
require 'class/CharacterManager.php';
require 'class/User.php';
require 'class/UserManager.php';
 
$User = new User;
$User->setLogin('');
$User->setPassword(md5(''));
 
$UserManager = new UserManager($User);
$UserManager->Logon();
 
try{
    if($User->getLogged() == 1){
 
        $Character = new Character;
        $Character->setIdFromSession();
        $CharacterManager = new CharacterManager($Character);
        $CharacterManager->upgradeStatistic('strenght', 5);
        $CharacterManager->setStatistics();
        echo $Character->getStrenght();
    }else{
        throw new Exception('Nie jestes zalogowany !');
    }
 
 
}catch(Exception $error){
    echo 'Wystapil blad:'.$error->getMessage();
 
}
 
 
 
?>
[PHP] pobierz, plaintext 

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ć

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ć.



Moja propozycja: Olej całkowicie logowanie. Skup się na problemie który twój program ma rozwiązywać.



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?

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).



Prawdę mówiąc nie wiem czy złapałem o co chodzi

mysle ze mozesz sobie poczytac moj post, zadawalem niemal ze identyczne pytania a dyskusja sie rozwinela calkiem na bogato.
Temat: prawidlowe OOP struktura klasy

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.

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.

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...

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.



Poczytam o tym



Ok, poprawię to. Zresztą miałem tak robić ale nie wiem czemu z tego zrezygnowałem



Własnie wiem, że aktualnie jestem taką papugą Twój post zresztą dosyć sporo pomógł mi w zrozumieniu jakie klasy przygotowywać aby miało to ręce i nogi, postaram się zastosować do tego. Dzięki.

Ogólnie wasze posty bardzo mi pomogły, dziękuję za zainteresowanie moim tematem mimo tego, że było już takich 2131545 Mam kolejne pytania:
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?

jeżeli masz dane w tablicy o takich samych nazwach jak pola to możesz zrobić sobie metodę setFromArray zamiast tego:

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'?



Tak ale nazwa tego obiektu nie mówi o tym co obiekt robi.



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?

Brakowało mi zamiennika nazwy Dzięki, po raz kolejny.



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ć ?



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ć.

[PHP] pobierz, plaintext 
<?php
class UserAuthoristation {
 
    function __construct($user) {
        $this->_user = $user;
 
        $this->_db = new Database();
        $this->_db->connect();
    }
 
    private $user;
 
    public function Logon(){
 
        $q = $this->_db->_pdo->prepare('SELECT * FROM user_account WHERE login = :login AND password = :password');
        $q->execute(array(':login' => $this->_user->getLogin(), ':password' => $this->_user->getPassword()));
 
        try{
 
             if($user = $q->fetch()){
                 echo "Zostales zalogowany";
                 $this->_user->setId($user[id]);
                 $this->_user->setLogged(1);
                 $this->_user->setSession();
             }
 
                else{
                        throw new Exception("Nie ma Cie w bazie ! Nie zostales zalogowany :(");
                    }
 
        }catch(Exception $error){
            echo $error->getMessage();
        }
 
    }
 
}
?>
 
[PHP] pobierz, plaintext 

Dodałem klase Database do obsługi bazy danych, za wiele w niej nie ma

[PHP] pobierz, plaintext 
 
class Database {
 
    function __construct() {
 
    }
 
    public function connect(){
        try{
        $this->_pdo = new PDO('mysql:host=localhost;dbname=;', '', '');
        $this->_pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        }catch(PDOException $error){
            echo 'Problem z baza :( :' . $error->getMessage();
        }
    }
 
}
 
 
[PHP] pobierz, plaintext 

Zmiany w klasie UserManager, dodałem funkcję tworzącą użytkownika, wywalilem z niej funkcje logującą i dodałem wykorzystanie klasy Database.

[PHP] pobierz, plaintext 
class UserManager {
 
    function __construct($user) {
        $this->_user = $user;
        $this->_db = new Database;
        $this->_db->connect();
    }
 
    private $_user;
 
    public function create(){
        $q = $this->_db->_pdo->prepare('INSERT INTO user_account VALUES(null,:login,:nick,:password,:email)');
        $q->execute(array(':login' => $this->_user->getLogin(), ':password' => $this->_user->getPassword(), ':nick' => 'test', 'email' => 'test'));
 
        return "User zostal stworzony";
    }
 
 
}
 
[PHP] pobierz, plaintext 

W index.php dorzuciłem tworzenie usera.

[PHP] pobierz, plaintext 
 
<?php
session_start();
 
require 'class/Character.php';
require 'class/CharacterManager.php';
require 'class/User.php';
require 'class/UserManager.php';
require 'class/Database.php';
require 'class/UserAuthorisation.php';
 
//logowanie
 
$User = new User;
$User->setLogin('Fanatyko');
$User->setPassword(md5('beniamins9'));
 
$UserAuthorisation = new UserAuthoristation($User);
$UserAuthorisation->Logon();
 
// zwiekszanie statystyk jesli jest sie zalogowanym
try{
    if($User->getLogged() == 1){
 
        $Character = new Character;
        $Character->setIdFromSession();
        $CharacterManager = new CharacterManager($Character);
        $CharacterManager->upgradeStatistic('strenght', 5);
        $CharacterManager->setStatistics();
        echo $Character->getStrenght();
    }else{
        throw new Exception('Nie jestes zalogowany !');
    }
 
 
}catch(Exception $error){
    echo 'Wystapil blad:'.$error->getMessage();
 
}
 
//tworzenie nowego usera
 
$User2 = new User;
$User2->setLogin('Test');
$User2->setPassword(md5('test'));
 
$UserManager2 = new UserManager($User2);
$UserManager2->create();
 
 
?>
[PHP] pobierz, plaintext 

Jest dobrze ?

Dla mnie kod wygląda całkiem znośnie, ale:
1. Unikaj takiego kodu

[PHP] pobierz, plaintext 
}catch(Exception $error){
    echo 'Wystapil blad:'.$error->getMessage();
 
}
[PHP] pobierz, plaintext 

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.

[PHP] pobierz, plaintext 
if($User->getLogged() == 1){
[PHP] pobierz, plaintext 

Czepiam się, ale bardziej naturalne wydaje mi się, że funkcja getLogged powinna powinna być funkcją typu isLogged i zwracać wartość boolean.

3.UserManager.php

[PHP] pobierz, plaintext 
    public function create(){
        $q = $this->_db->_pdo->prepare('INSERT INTO user_account VALUES(null,:login,:nick,:password,:email)');
        $q->execute(array(':login' => $this->_user->getLogin(), ':password' => $this->_user->getPassword(), ':nick' => 'test', 'email' => 'test'));
 
        return "User zostal stworzony";
    }
[PHP] pobierz, plaintext 

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

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

@Fantazyn

dzięki za odpowiedź, naprawdę mi się przydała. Jakoś nie pomyślałem o logach, nie wiem czemu . W niedługim czasie poprawię po raz kolejny mój kod

@banditpanda

poprawię to

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

[PHP] pobierz, plaintext 
$kucharz = new Kucharz(array($skladnik, $skladnik)) 
$pizza = $kucharz->robPizze('Roma')
[PHP] pobierz, plaintext 

piszesz

[PHP] pobierz, plaintext 
 
$pizza = new Pizza();
$pizza->setSkladnik1($skladnik);
$pizza->setSkladnik2($skladnik);
$kucharz = new Kucharz()
$kucharz->setPizzaName('Roma')
$kucharz->setPizza($pizza);
$pizza = $kucharz->getPizza($pizza);
[PHP] pobierz, plaintext 

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 :

[PHP] pobierz, plaintext 
$User = new User;
$User->setLogin('');
$User->setPassword(md5(''));
[PHP] pobierz, plaintext 

to powinienem to zrobić tak:

[PHP] pobierz, plaintext 
$User = new User('haslo', 'login')
[PHP] pobierz, plaintext 

i wtedy konstruktorze wykorzystać settery czy normalnie przypisać do zmiennych ?




Bez tego obiektu nie mam czego wstrzyknać do klas UserManager, UserAuthorisation. Coś muszę im przekazywać, przynajmniej tak mi się zdaję.

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 .

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