Good Practice Object Orientation

Question

I have a class called clients. I have 2 methods called verificaCPF() and queryInsert().

Both methods when it makes the instruction it gives a return (is working normal). My question is whether this form I’m doing is a good practice? Do you do this? Can you give me a hint to improve that return part of the instruction?

Thank you very much ;)

 public function verificaCPF($cpf) {
    try {
        $this->cpf = $cpf;
        $stmt = $this->conn->conexao()->prepare("SELECT cli_cpf FROM clientes WHERE cli_cpf =:cli_cpf");
        $stmt->bindParam(":cli_cpf", $this->cpf);
        $stmt->execute();
        if($stmt->rowCount() <=0){
            return 'ok'; //ALGUMA DICA PRA SER MAIS LEGIVEL PRA QUEM TA DE FORA?
        }else{
            return 'nao';
        }

    } catch (Exception $ex) {
        echo $ex->getMessage();
    }

index php.

if (isset($_POST['cadCliente'])) {
$objCli = new clientes();

if($objCli->verificaCPF($_POST['cpf']) =='nao'){
    echo '<script>alert("CPF EM DUPLICIDADE");</script>';
}else{
    if ($objCli->queryInsert($_POST) == 'ok') {
    echo '<script>alert("Cadastro realizado!");</script>';
}
}

Answer 1

I hate to hear the word "good practice" as if it were a pattern to be followed to the letter. There are times when each case is a case.

But in your particular case, wouldn’t it be too complicated to return a string just to return a status in a method?

In your case, there are two possible answers: 'ok' and 'no'. That’s already so, why not return "true or false". Wouldn’t it be easier to return a boolean type (true or false)?

As such, it is neither "good practice" but common sense.

Want to see if the condition is true or false? Return the boolean.

Also, do not treat the exception within the call of a printing method with a echo. Leave to deal with the exception on external call. That’s what I would do in this case.

Even, if memory serves, it’s a recommendation from PSR, separate data return output.

I would change the method to the following way:

/**
* @throws PDOException
* @param string $cpf
* @return boolean
*/
public function verificaCPF($cpf) {

    $this->cpf = $cpf;
    $stmt = $this->conn->conexao()->prepare("SELECT cli_cpf FROM clientes WHERE cli_cpf =:cli_cpf");
    $stmt->bindParam(":cli_cpf", $this->cpf);
    $stmt->execute();

    return $stmt->rowCount() > 0;

}

Note that I simply returned $stmt->rowCount() > 0. This means I am returning a boolean type. It will return true whenever rowCount() is greater than 0.

Then you just have to treat things simply and clearly:

if (isset($_POST['cadCliente'])) {

    $objCli = new clientes();


    try{

        if (! $objCli->verificaCPF($_POST['cpf']) ){

            echo '<script>alert("CPF EM DUPLICIDADE");</script>';

        } else {

            echo '<script>alert("Cadastro realizado!");</script>';
        }


    } catch (\Exception $e) {

        echo $e->getMessage();
    }

}

And finally, maybe not for the sake of standards, but for the sake of common sense, I would rename the method verificarCpf, because he does not make it clear that he checks whether a CPF is duplicated. Maybe a verificarCpfDuplicado caiba, but it goes from you.

Always try to make your code as clear as possible. What I do is always imagine that a person will see my code and that they need to understand it without having to read comments.