Sistema de login con POO

[CodeFactory]

Yo soy programador de java, obviamente orientado a objeto y siempre que he programado en php lo he hecho secuencialmente, por lo tanto, he decidido a programar PHP POO y aquí os dejo un sistema de login que he hecho en un momentito para que me digáis vuestras opiniones.

Código PHP:

  <?php
interface Login{
    public function verificar();
}

class LoginImpl implements Login{
    private $username;
    private $password;
    private $dato = array();
    private $db;
    
    private $consulta;

    public function __construct($username,$password,$datos){
        $this->username = $username;
        $this->password = $password;
        
        $this->dato = explode(' ',$datos);
        $this->db = mysqli_connect($this->dato[0],$this->dato[1],$this->dato[2],$this->dato[3]);
    }

    public function verificar(){
        $res = false;
        $this->consulta = mysqli_query($this->db, "SELECT username,password FROM usuarios WHERE username = '".mysqli_real_escape_string($db, $this->username)."' AND password = '".mysqli_real_escape_string($db, $this->password)."' LIMIT 0,1");
        if($consulta1 = mysqli_fetch_assoc($this->consulta)){
            $res = true;
        }
        return $res;
    }
    
}

$login = new LoginImpl("usuario","password","localhost root user db");
if($login->verificar() == true){
    echo 'Correcto';
}else{
    echo 'Incorrecto';
}
?>   

Saludos.

[Malenko]

Para empezar, no haces chequeos de errores. Si falla la conexión al realizar el connect no tendrías ni que intentar lanzar la select. Esto me lleva al otro error... el constructor no tendría que realizar el connect ya que estarás ocupando una conexión hasta que el desarrollador llame al "verificar". Y tampoco has liberado el recurso (la conexión) al dejar de usarla.

Personalmente, los datos de usuario y contraseña a verificar los pasaría en la función "Verificar" y no en el constructor.

[CodeFactory]

Iniciado por Malenko

Para empezar, no haces chequeos de errores. Si falla la conexión al realizar el connect no tendrías ni que intentar lanzar la select. Esto me lleva al otro error... el constructor no tendría que realizar el connect ya que estarás ocupando una conexión hasta que el desarrollador llame al "verificar". Y tampoco has liberado el recurso (la conexión) al dejar de usarla.

Personalmente, los datos de usuario y contraseña a verificar los pasaría en la función "Verificar" y no en el constructor.

La conexion y los datos de usuario(al menos) si debería de hacerla en el constructor puesto que después puedo crear mas métodos y utilizarlos.

Si por algun motivo no utilizo el método verificar() entonces según tu forma de verlo, nunca se haría la conexion ni obtendría el valor del usuario.
PD: Me falto un método para cerrar la conexión.

PD1: La duda que yo tengo es, ¿Seria mejor crear la consulta en la clase o fuera de la clase?

Código PHP:

  <?php
interface Login{
    public function verificar();
}

class LoginImpl implements Login{
    private $username;
    private $password;
    private $dato = array();
    private $db;
    
    private $consulta;

    public function __construct($username,$password,$datos){
        $this->username = $username;
        $this->password = $password;
        
        $this->dato = explode(' ',$datos);
        $this->db = mysqli_connect($this->dato[0],$this->dato[1],$this->dato[2],$this->dato[3]);
    }

    public function verificar(){
        $res = false;
        $this->consulta = mysqli_query($this->db, "SELECT username,password FROM usuarios WHERE username = '".mysqli_real_escape_string($db, $this->username)."' AND password = '".mysqli_real_escape_string($db, $this->password)."' LIMIT 0,1");
        if($consulta1 = mysqli_fetch_assoc($this->consulta)){
            $res = true;
        }
        return $res;
    }

    public function close(){
        mysqli_close($this->db);
    }
    
}

$login = new LoginImpl("usuario","password","localhost root user db");
if($login->verificar() == true){
    echo 'Correcto';
}else{
    echo 'Incorrecto';
}
$login->close();
?>   

Saludos.

[Malenko]

Si vas a agregar más métodos a la clase que requieran conexión contra la BBDD y justifiquen mantener la conexión abierta, entonces tiene sentido dejarla en el constructor. Pero has pedido que te comenten el código que has puesto, no el que quizás en un futuro hagas. Y ahora mismo en ningún momento liberas la conexión.

Por otro lado, si estás realizando POO y quieres seguir los principios SOLID tendrías que desacoplar el acceso a BBDD de la clase de Login y permitir poder intercambiar de un SGBD a otro, etc.

[xerifandtomas]

Opino igual que el compañero, la conexión a la bd debería de esta en una clase independiente.

En cuanto a tu pregunta

La duda que yo tengo es, ¿Seria mejor crear la consulta en la clase o fuera de la clase?

Sí el método o clase tiene como única finalidad hacer esa consulta, no creo que tenga mucho sentido poder permitir pasarle al método cualquier consulta construida fuer de la clase. Por lo que yo diría que esta bien así. Aunque lo haría en una variable antes del query por legibilidad, no me gustan las líneas infinitas, jaja.

[CodeFactory]

Iniciado por Malenko

Si vas a agregar más métodos a la clase que requieran conexión contra la BBDD y justifiquen mantener la conexión abierta, entonces tiene sentido dejarla en el constructor. Pero has pedido que te comenten el código que has puesto, no el que quizás en un futuro hagas. Y ahora mismo en ningún momento liberas la conexión.

Por otro lado, si estás realizando POO y quieres seguir los principios SOLID tendrías que desacoplar el acceso a BBDD de la clase de Login y permitir poder intercambiar de un SGBD a otro, etc.

Iniciado por xerifandtomas

Opino igual que el compañero, la conexión a la bd debería de esta en una clase independiente.

En cuanto a tu pregunta

Sí el método o clase tiene como única finalidad hacer esa consulta, no creo que tenga mucho sentido poder permitir pasarle al método cualquier consulta construida fuer de la clase. Por lo que yo diría que esta bien así. Aunque lo haría en una variable antes del query por legibilidad, no me gustan las líneas infinitas, jaja.

Muchas gracias por responder a los 2.
"Por otro lado, si estás realizando POO y quieres seguir los principios SOLID tendrías que desacoplar el acceso a BBDD de la clase de Login y permitir poder intercambiar de un SGBD a otro, etc."

¿Te refieres a que ponga la conexión en una clase propia para poder implementarla a todas las demás clases y no tener que crear la conexión en todas las clases cierto?

Saludos.

PD: Acabo de entender lo que me dijiste, muchas gracias.

[Malenko]

La idea es que si cambias de motor de BBDD no tengas que tocar la clase Login.