Autor Zpráva
larryx
Profil
Nazdar, zacinam sprtat do OOP, vytvoril som taku jednoduchu triedu na prihlasenie a chcem sa opytat tych skusenejsich ci sa vlastne uberam spravnym smerom, ci je to dobre "vymyslene" pre OOP, dakujem za reakcie....
<?php

class user {
    public $showForm=true;
    public $mysql;

    public function  __construct($conn) {
        $this->mysql=$conn;
        if(isset($_SESSION["login"]) and isset($_SESSION["password"])){
            $result=$this->mysql->query("SELECT `id` FROM `users` WHERE `login`='".$_SESSION["login"]."' and `password`='".$_SESSION["password"]."'");
            if($result->num_rows == 1){
                $this->showForm=false;
            } else {
                $this->logout();
            }
        }
    }
    public function userMenu(){
        if($this->showForm === true){
            echo '<form method="post">'."\n\t".
                 'login: <input type="text" name="login"> '."\n\t".
                 'password: <input type="text" name="password">'."\n\t".
                 '<input type="submit">'."\n\t".
                 '</form>'."\n";
        } elseif($this->showForm === false){
            echo "Prihlásený: ".$_SESSION["login"].
                 ' <a href="?logout">odhlásiť</a>'."\n";
        }
    }
    public function login(){
        if(isset($_POST["login"]) and isset($_POST["password"])){
            $user=$this->mysql->query("SELECT `password` FROM `users` WHERE `login`='".$_POST["login"]."'");

            if($user->num_rows == 1){
                $pass=$user->fetch_assoc();
                if($pass["password"] == $_POST["password"]){
                    $_SESSION["login"]=$this->mysql->real_escape_string($_POST["login"]);
                    $_SESSION["password"]=$this->mysql->real_escape_string($_POST["password"]);
                    header("Location:".$_SERVER["HTTP_REFERER"]);
                } else{
                    header("Location: ?errno=1");
                }
            } else{
                    header("Location: ?errno=1");
            }
        }
    }
    public function logout(){
        unset($_SESSION["login"]);
        unset($_SESSION["password"]);
        header("Location:".$_SERVER["HTTP_REFERER"]);
    }
}
?>
AM_
Profil
z hlediska kódu to asi funguje, z hlediska návrhu nic moc. třída user by měla obsahovat informace o uživateli jako vlastnosti a různé akce s ním proveditelné jako metody, tedy:
- proměnná showForm nemá ve třídě user moc co dělat. Spíš by měl mít metodu isLogged(), která by ověřila, zda je přihlášen, a na základě toho by se v šabloně vypsal přihlašovací nebo "odhlašovací" form.
- jinak řečeno, funkce userMenu zde taky nemá moc co dělat, toto patří do šablony aplikace, nikoli do metody třídy
- proměnná mysql by rozhodně neměla být public, spojení s databází není vlastností uživatele
- konstruktor a metodu "login" bych sjednotil (obě de facto dělají to samé - ověřují správnost přihlášení, jen každá má jiný zdroj dat)
larryx
Profil
dakujem za pripomienky, uz som to poriesil tak ako si navrhol a je fakt ze je to takto lepise (aj katsie) este nejake pripomienky ???
<?php

class user {
    private $mysql;

    public function  __construct($conn) {
        $this->mysql=$conn;
        if(isset($_POST["login"]) and isset($_POST["password"])){
            $user=$this->mysql->query("SELECT `password` FROM `users` WHERE `login`='".$_POST["login"]."'");
            if($user->num_rows == 1){
                $pass=$user->fetch_assoc();
                if($pass["password"] == $_POST["password"]){
                    $_SESSION["login"]=$this->mysql->real_escape_string($_POST["login"]);
                    $_SESSION["password"]=$this->mysql->real_escape_string($_POST["password"]);
                    header("Location:".$_SERVER["HTTP_REFERER"]);
                } else{
                    header("Location: ?errno=1");
                }
            } else{
                    header("Location: ?errno=1");
            }
        }
    }
    public function isLogged(){
        if(isset($_SESSION["login"]) and isset($_SESSION["password"])){
            $result=$this->mysql->query("SELECT `id` FROM `users` WHERE `login`='".$_SESSION["login"]."' and `password`='".$_SESSION["password"]."'");
            if($result->num_rows == 1){
                return true;
            } else {
                return false;
            }
        } else {
            return false;
        }
    }
    public function logout(){
        unset($_SESSION["login"]);
        unset($_SESSION["password"]);
        header("Location:".$_SERVER["HTTP_REFERER"]);
    }
}
?>
Joker
Profil
larryx:
ci je to dobre "vymyslene" pre OOP
Moc ne. Nepřipadá mi to jako třída, spíš jako funkce nějak se týkající uživatelů uzavřené do jednoho bloku.

Například je to třída pro uživatele, přitom nemá žádné atributy týkající se toho uživatele (jméno například), zato má atributy, které bych tam nečekal.
Snažil bych se tu třídu udělat víc uzavřenou.
Výhodou objektů má být mj. znovupoužitelnost kódu. Proč je objekt uživatele tak svázaný s POST a SESSION?
Asi raději než natvrdo tahat data z $_POST bych tu metodu udělal obecnější, aby měla jméno a heslo jako parametry.
Dál místo ukládání do session bych si udělal atributy objektu a ukládal to do nich. V session by případně mohl být celý ten objekt.

AM:
konstruktor a metodu "login" bych sjednotil
- čímž ani nebude potřebovat identifikátor databázového spojení, protože to bude stačit jako parametr té metody.
AM_
Profil
Joker:
čímž ani nebude potřebovat identifikátor databázového spojení
ten bych nezahazoval, později se může binding s databází hodit např. pro úpravu uživatelského účtu.

larryx:
ve funkci isLogged se zbytečně dotazuješ databáze, zda je uživatel přihlášen, když to už jsi pro daný objekt ověřil v konstruktoru. Jinak to co píše Joker, je to pořád takové hrubé OOP, skutečné modelování objektů je trochu jiná liga, ale pro začátek to není tak zlé, viděl jsem mnohem horší kódy :)

Vaše odpověď

Mohlo by se hodit


Prosím používejte diakritiku a interpunkci.

Ochrana proti spamu. Napište prosím číslo dvě-sta čtyřicet-sedm:

0