Jump to content

Recommended Posts

As much as I know about what I know, I know even less about OOP so I figured it is time I learn. I have read enough to start having questions.

 

From what I have seen, a class should have a single purpose.

 

Using a registration/login example, it would seem to me that I would have at least class Register and class Login since each one is a particular use, correct?

 

Since a registration is an act of CRUD, how then would that fit in if you had a CRUD class? Is the register class not needed?

 

Also, at the base of everything is the DB connection. Since PDO is a class, I am confused when I see class Database with the connection code. Isnt this not necessary?

 

What really should be in a User class? If you were to get a list of users, that is still a CRUD act so what would really go in a user class if we are sticking to single use?

 

Also, would I not have a separate password class to do hashing/verification etc.?

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/
Share on other sites

Basically, a class should represent a noun. It is responsible for a particular thing. Inside the class are methods and those represent verbs. Those methods perform actions.

 

$noun = new Noun();
$noun->verb();

Using a registration/login example, it would seem to me that I would have at least class Register and class Login since each one is a particular use, correct?

Registering and logging in are verbs. Ask yourself "what is doing the registering and what is doing the logging in?" The answer is a user. (Which is a noun.) So there should be a user class with methods to register and to log in.

 

Since a registration is an act of CRUD, how then would that fit in if you had a CRUD class? Is the register class not needed?

Two answers:

1. Now you're thinking about models. A model is a class, which means it's a noun.

2. Does it make sense to "create" a registration? "Create" a login? Not really.

 

Registration is the act of creating a new user. So there should be a class for the user model which has CRUD operations.

 

Logging in doesn't really impact the model... unless you wanted to record login time or last activity, then that would have an impact of updating the user record. But otherwise you're looking up information, which is part of CRUD.

 

Also, at the base of everything is the DB connection. Since PDO is a class, I am confused when I see class Database with the connection code. Isnt this not necessary?

Depends. Most of the time a database class represents a database connection (a noun), and has methods to help you do stuff so that you don't have to do everything manually via PDO. Such as

a) Abstracting the database connector away (which PDO kinda does already)

b) Retrieving database credentials so the rest of your code doesn't need to know

c) Helper methods to make some operations easier, like executing a SELECT and returning the results in an array

 

What really should be in a User class?

Well, what kinds of things does a user do? We've already covered "a user can register" and "a user can log in".

 

If you were to get a list of users, that is still a CRUD act so what would really go in a user class if we are sticking to single use?

"Single" is about making sure a class isn't responsible for multiple things that don't make sense to combine. Like a class that does database operations as well as handle file uploads. If you can say "this class is responsible for " then you're probably okay.

 

Also, would I not have a separate password class to do hashing/verification etc.?

I could argue in favor of doing that, but practically speaking

 

1. There aren't too many operations involved with passwords: create new random password, create hash of password, verify password

2. Often those operations are part of a larger operation: create a new random password because the user forgot theirs, or create a hash of a password because the user is changing theirs, or verify a password because a user is trying to log in

3. There's not much code involved

3. Pretty much everything you do with a password has to do with a user, for which you already (hypothetically) have a class, so you might as well put the password code in there

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532074
Share on other sites

No dissent, just a comment:

 

When you have to choose between a single complex class (like User) and multiple simple classes (PasswordHash, Authentication, ...), I'd generally pick the latter. A “dumb” class which only takes care of one aspect is easier to understand, easier to test and more future-proof.

 

Having a dedicated class for password hashes may sound like overkill, but there's actually more to it than creating and verifying a hash. You'll also want a parser to check the hash properties, and you have to identify obsolete hashes, analogous to the password_needs_rehash() function. In addition to that, password hashing is subject to change. I've revised my own code a couple of times, and one of the next PHP releases will introduce a more modern algorithm like Argon2. These changes shouldn't require you to mess with the inner workings of your User class. It's much safer to edit an isolated PasswordHash class while leaving everything else alone.

 

It's the same thing with, say, user authentication. You may start with a simple passwork check, but that's not necessarily all you'll ever need. What about brute-force protection? Public-key authentication for high-value accounts? If you have a separate Authentication class, making those changes is not a problem. If everything is embedded into a User class, it's more difficult.

 

So personally, I would create one PasswordHash class, one Authentication class, maybe a Session class and finally a User class. It's perfectly fine if each of them only contains a few lines of code.

Edited by Jacques1
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532083
Share on other sites

@Jaques1, what you are saying makes more sense to me, especially considering SOLID. The more I researched, the more it seemed that most everything I saw on oop users registration/login wasn't best practice which is why I came here for guidance so I can learn this right the first time.

 

To me, if everything is a single purpose class, I would have at least the following:

class Database (The  DB Connection)

class Register (insert user/handle duplicate username/email)

class Password (function password_verify(), function password_hash(), function password_rehash())

class Error (various error messages???)

class Login (Handles the login/ login failures, login logging, perhaps logout as well)

class Reset (Handles the password reset/password updating: utilizing the class Password)

class Email (Send out various emails, ie: validate email, password change notification, password reset email, etc..)

 

If this is right, which class extends who? who is not extended but "included"? (I know that is not right, dont know OOP speak for it yet) Aside from a dependence on the DB connection, shouldn't a class pretty much be a stand alone program not dependent on the other classes for the most part?

 

I have seen the "All in one" classes but they just dont seem right considering SOLID and if I am not mistaken, are referred to as a "GOD Class". If we are building the ultimate best practice OOP user reg/login would we go about it with all these classes even though some may have minimal code? If I understand correctly, doing unit testing is much easier with this kind of separation as well. Single responsibility no matter how small seems very right to me.

Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532084
Share on other sites

For the record: There's nothing inherently wrong with having a single User class. That's not a God Object, because it has a specific purpose (representing an application user). The discussion is more about how fine-grained the class structure should be.

 

And you shouldn't create a new class for every single task. The registration and a password reset are really just simple create/update operations in the User and Authentication class. It wouldn't make sense to keep them separate.

 

Of course there's more than one way, but I would do something like this:

  • if plain PDO isn't enough: a Database class
  • PasswordHash: create($password, $parameters), check($password), parse($hash), needsUpdate($desiredParameters), ...
  • Authentication: create($username, $password), check($username, $password), update($username, $currentPassword, $newPassword), createResetToken($username), resetPassword($username, $resetToken, $newPassword)
  • Session: create($userId), resume(), terminate() [the user is logged out]
  • User: representing the aspects of the account which aren't security-related (personal data etc.)

Creating various e-mails can be solved with templates. I wouldn't necessarily use a separate Email class.

 

The above classes shouldn't do too much. For example, Authentication::createResetToken() should really just generate a random token, store it and return it to the caller. Sending out an e-mail/SMS/... should be handled at a higher level, e. g. the controller.

 

 

 

If this is right, which class extends who? who is not extended but "included"? (I know that is not right, dont know OOP speak for it yet) Aside from a dependence on the DB connection, shouldn't a class pretty much be a stand alone program not dependent on the other classes for the most part?

 

Using an object in another object is called association, and it's perfectly normal. You can avoid hard-coded class dependencies with Dependency Injection.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532086
Share on other sites

Starting to get a grasp. Dependency Injection was the unknown I was looking for. Have yet to see if I would have need for a Container/IOC. What I was seeking is a real separation of concerns. I was able to accomplish it quite well in procedural with templates.

 

Here is what I have so far. Any comments or improvements before I move on?

<?php
$dbhost  = 'localhost';
$dbname  = 'meetmarket_development';
$dbuser  = 'root';
$dbpass  = '';
$charset = 'utf8';
$dsn     = "mysql:host=$dbhost;dbname=$dbname;charset=$charset";
$opt = [
    PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
    PDO::ATTR_EMULATE_PREPARES   => false,
];

$pdo = new PDO($dsn, $dbuser, $dbpass, $opt);


class User
    {
    /**
     * @var PDO The connection to the database
     */
    protected $pdo;

    /**
     * Construct.
     * @param PDO $pdo The database connection
     */
    public function __construct($pdo)
        {
        $this->pdo = $pdo;
        }

    /**
     * @param $request: $_POST or $_GET
     * @param $columns: Columns to SELECT
     */
    public function select_user($request, $columns)
        {
        $sql  = "SELECT ";
        foreach ($columns AS $field){
            $sql .= $field.', ';
        }
        $sql = rtrim($sql, ", ");
        $sql .=' FROM users WHERE username = ?';
        $stmt = $this->pdo->prepare($sql);
        $stmt->execute(array($request));
        $row = $stmt->fetch(PDO::FETCH_ASSOC);
        echo "<pre>";
        print_r($row);
        echo "</pre>";
        }
    }

$request = $_POST['username'] = 'myusername';

$user           = new User($pdo);
$columns = array('username', 'password');
$user->select_user($request, $columns);
?>

RESULT

Array
(
    [username] => myusername
    [password] => $2y$10$72JVve7WJCctEuB1SWA81OBItahuCuh9bWF/vI5NWTA1siFU9f8U6
)
Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532137
Share on other sites

The User class should behave like a model. That is, one instance represents one user, all public attributes (not including the password hash and the reset token) are loaded upon instantiation and can be accessed through getter methods. I find the current approach a bit confusing, because a User, well, isn't a user. It's more like a database of all users, and you have to constantly specify which user you mean.

 

You should also use type declarations in your methods to make sure that the $pdo argument is in fact a PDO instance.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532139
Share on other sites

Not sure what all you said means, yet. The purpose of the particular function select_user would be used as part of a user logging in or for a particular user profile and is only part of the functionality that will be in the class.

 

ie;

Select user, password

Compare password hash

Login Valid User

 

 

I figure what this class should initially be able to do is login a user, do password reset/change, possibly register/add new user and list all users, deactivate users (for the admin)

Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532141
Share on other sites

A separate class for a single query is a bit too fine-grained. I'd put the query into the Auth class and reserve the User class for general user-related data and tasks (like the name suggests).

 

Either way, don't expect to get the perfect OO design on the first attempt. Even programmers with years of OOP experience argue about the ideal approach. You've already skipped many stages that people usually go through (the god object, hard-coded class references etc.). Now I'd simply try out different approaches, analyze the problems and look for common solutions.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532143
Share on other sites

Right now I am just trying to make something work to grasp OOP. Right now the name of anything really doesn't matter.

 

Here is my immediate goal:

Create a Loosely coupled DB connected class - DONE

Compare user supplied password to DB password hash

Set a logged in session for a valid user.

 

Where I currently am:

I can get the username and password

 

Where I am now stuck:

comparing user supplied password to password hash from the result of select_user. I need to pass the hashed password from select_user to function verify_password. As of the moment I dont know how to pass results between methods.

 

Update Code:

<?php
//----------------------------------------------------------------------------
// Database Connection
//----------------------------------------------------------------------------

$dbhost  = 'localhost';
$dbname  = 'meetmarket_development';
$dbuser  = 'root';
$dbpass  = '';
$charset = 'utf8';
$dsn     = "mysql:host=$dbhost;dbname=$dbname;charset=$charset";
$opt = [
    PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
    PDO::ATTR_EMULATE_PREPARES   => false,
];

$pdo = new PDO($dsn, $dbuser, $dbpass, $opt);

//----------------------------------------------------------------------------
// Class User
//----------------------------------------------------------------------------

class User
    {
    /**
     * @var PDO The connection to the database
     */
    protected $pdo;
    protected $row;

    /**
     * Construct.
     * @param PDO $pdo The database connection
     */
    public function __construct($pdo)
        {
        $this->pdo = $pdo;
        }

    /**
     * @param $request: $_POST or $_GET
     * @param $columns: Columns to SELECT
     */
    public function select_user($request, $columns)
        {
        $sql = "SELECT ";
        foreach ($columns AS $field)
            {
            $sql .= $field . ', ';
            }
        $sql = rtrim($sql, ", ");
        $sql .= ' FROM users WHERE username = ?';
        $stmt = $this->pdo->prepare($sql);
        $stmt->execute(array(
            $request
        ));
        $row = $stmt->fetch(PDO::FETCH_ASSOC);
        return $row;
        }

    public function verify_password($password)
        {
        echo $password;// User supplied password makes it to here
        echo $this->row['password'];// No Data


        if (password_verify($password, $this->row['password']))
            {
            echo 'Valid User';// Set $_SESSION['logged_in'];    
            }
        else
            {
            echo 'Bad';
            }
        }
    } // End Class User

//----------------------------------------------------------------------------
//
//----------------------------------------------------------------------------

$request  = $_POST['username'] = 'myusername';
$password = 'pass';

$user    = new User($pdo);
$columns = array(
    'username',
    'password'
);
$user->select_user($request, $columns);
$user->verify_password($password);
?>
Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532145
Share on other sites

Ok, figured it out. Not sure if it is correct though.

<?php
//----------------------------------------------------------------------------
// Database Connection
//----------------------------------------------------------------------------

$dbhost  = 'localhost';
$dbname  = 'meetmarket_development';
$dbuser  = 'root';
$dbpass  = '';
$charset = 'utf8';
$dsn     = "mysql:host=$dbhost;dbname=$dbname;charset=$charset";
$opt = [
    PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
    PDO::ATTR_EMULATE_PREPARES   => false,
];

$pdo = new PDO($dsn, $dbuser, $dbpass, $opt);

//----------------------------------------------------------------------------
// Class User
//----------------------------------------------------------------------------

class User
    {
    /**
     * @var PDO The connection to the database
     */
    protected $pdo;
    protected $row;

    /**
     * Construct.
     * @param PDO $pdo The database connection
     */
    public function __construct($pdo)
        {
        $this->pdo = $pdo;
        }

    /**
     * @param $request: $_POST or $_GET
     * @param $columns: Columns to SELECT
     */
    public function select_user($request, $columns)
        {
        $sql = "SELECT ";
        foreach ($columns AS $field)
            {
            $sql .= $field . ', ';
            }
        $sql = rtrim($sql, ", ");
        $sql .= ' FROM users WHERE username = ?';
        $stmt = $this->pdo->prepare($sql);
        $stmt->execute(array(
            $request
        ));
        $row = $stmt->fetch(PDO::FETCH_ASSOC);
        $this -> username = $row['username'];
        $this -> hash = $row['password'];
        //return $row;
        }

    public function verify_password($password)
        {
        echo $password;// User supplied password makes it to here
        echo $this->hash;// GOT DATA!

        if (password_verify($password, $this->hash))
            {
            echo 'Valid User';// Set $_SESSION['logged_in'];
            }
        else
            {
            echo 'Bad';
            }
        }
    } // End Class User

//----------------------------------------------------------------------------
//
//----------------------------------------------------------------------------

$request  = $_POST['username'] = 'myusername';
$password = 'pass';

$user    = new User($pdo);
$columns = array(
    'username',
    'password'
);
$user->select_user($request, $columns);
$user->verify_password($password);
?>
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532146
Share on other sites

Current goals accomplished. Anyone see changes that should be made or problems with current code? OOP is making my brain hurt. Done for the day.

 

Not down on what should be public or private yet. From what I do understand I can make everything private. My understanding is that private/public has to do with being able to modify stuff outside the class. Protected? Not even going there at the moment.

 

 

LATEST CODE:

<?php
//----------------------------------------------------------------------------
// Database Connection
//----------------------------------------------------------------------------

$dbhost  = 'localhost';
$dbname  = 'meetmarket_development';
$dbuser  = 'root';
$dbpass  = '';
$charset = 'utf8';
$dsn     = "mysql:host=$dbhost;dbname=$dbname;charset=$charset";
$opt = [
    PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
    PDO::ATTR_EMULATE_PREPARES   => false,
];

$pdo = new PDO($dsn, $dbuser, $dbpass, $opt);

//----------------------------------------------------------------------------
// Class User
//----------------------------------------------------------------------------

class User
    {
    /**
     * @var PDO The connection to the database
     */
    protected $pdo;
    protected $row;
    protected $error;
    protected $password_hash;

    /**
     * Construct.
     * @param PDO $pdo The database connection
     */
    public function __construct($pdo)
        {
        $this->pdo = $pdo;
        }

    /**
     * @param $request: $_POST or $_GET
     * @param $columns: Columns to SELECT
     */
    public function select_user($request, $columns)
        {
        $sql = "SELECT ";
        foreach ($columns AS $field)
            {
            $sql .= $field . ', ';
            }
        $sql = rtrim($sql, ", ");
        $sql .= ' FROM users WHERE username = ?';
        $stmt = $this->pdo->prepare($sql);
        $stmt->execute(array(
            $request
        ));
        $row = $stmt->fetch(PDO::FETCH_ASSOC);

        // No match, Do stuff.
        if (!$stmt->rowCount())
            {
            $this->error['username'] = 'Bad User Name';
            }
        else
            {
            $this->username      = $row['username'];
            $this->password_hash = $row['password'];
            }
        } // End function select_user


    public function verify_password($password)
        {
        if (isset($this->password_hash))
            {
            if (password_verify($password, $this->password_hash))
                {
                echo 'Valid User'; // Set $_SESSION['logged_in'];
                }
            else
                {
                $this->error['password'] = 'Bad Password';
                }
            }
        }

    public function check_errors()
        {
        if (count($this->error) > 0)
            {
            echo 'Errors<br>';
            echo $error = implode("<br >\n", $this->error) . "\n";
            die;
            }
        } // End function check_errors

    } // End Class User

//----------------------------------------------------------------------------
//
//----------------------------------------------------------------------------

/*
CURRENT PROGRAM FLOW
1. Compare user submitted username to DB username. If match goto 2. else die with error
2. Username is valid, compare user submitted password to DB hashed password. If match, valid user, else die with error.
*/

$request  = $_POST['username'] = 'myusername';
$password = 'pass';

$user    = new User($pdo);
$columns = array(
    'username',
    'password'
);
$user->select_user($request, $columns);
$user->verify_password($password);
$user->check_errors();

?>
Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532147
Share on other sites

Names do matter, because they tell us (and you) what the class is supposed to do. So a User is now responsible for authentication?

 

The problem is that your class is little more than a container of functions. You've delegated the heavy lifting to the caller which has to execute the functions one after another, following an (unknown) protocol.

 

It should actually be the other way round: The class is responsible for performing the task. If I'm the caller, it's not my job to look up table fields in your database. Chances are I don't even know those fields. I just want to know if a specific username/password combination is valid, so I expect a method like this:

/**
 * Checks whether a username/password combination is valid
 *
 * @param string $username the username to be checked
 * @param string $password the corresponding password
 *
 * @return boolean the result of the check
 */
function check($username, $password)

Now it's up to the class to go through all necessary steps (query the database, hash the password etc.)

 

This is also what public/protected/private is about: A class has a public interface, consisting of all public methods and attributes. This is what gets exposed to the caller and what you would put into the documentation. Then there may be dozens of other methods and attribute which are only relevant for the inner workings of the class. This is what you make private (or protected), effectively hiding it from the caller.

 

I't generally a good idea to start with the public interface: How is the caller supposed to interact with the class? What are the features? Then you take care of the implementation.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532155
Share on other sites

Thanks for your input @Jaques1. As of now I have about 12 hours of serious OOP experience against 25+ years of procedural experience. As brief as it was, the code comment was extremely helpful. I cut out at least 50% of the previous code and really got down to a single use class as well as transferring the workload to the class.

 

Here is my revised code. By the way, would the logout function be part of this? What are your thoughts on this revision?

 

<?php

// ----------------------------------------------------------------------------
// Database Connection
// ----------------------------------------------------------------------------
 
$dbhost = 'localhost';
$dbname = 'meetmarket_development';
$dbuser = 'root';
$dbpass = '';
$charset = 'utf8';
$dsn = "mysql:host=$dbhost;dbname=$dbname;charset=$charset";
$opt = [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ];
$pdo = new PDO($dsn, $dbuser, $dbpass, $opt);
 
// ----------------------------------------------------------------------------
// Class AuthenticateUser
// ----------------------------------------------------------------------------
 
class AuthenticateUser
    {
    public function __construct($pdo)
        {
        $this->pdo = $pdo;
        }
 
    /**
     * Checks whether a username/password combination is valid
     *
     * @param string $username the username to be checked
     * @param string $password the corresponding password
     *
     * @return boolean the result of the check
     */
 
    public function check($username, $password)
        {
        $sql  = "SELECT username, password FROM users WHERE username = ?";
        $stmt = $this->pdo->prepare($sql);
        $stmt->execute(array(
            $username
        ));
        $row = $stmt->fetch(PDO::FETCH_ASSOC);
 
        if ($stmt->rowCount())
            {
            if (password_verify($password, $row['password']))
                {
                return $this;
                }
            }
        } // End function
    } // End Class
 
// ----------------------------------------------------------------------------
//
// ----------------------------------------------------------------------------
 
/*
CURRENT PROGRAM FLOW
1. Compare user submitted username & password to DB username & password.
2. If username and password valid return true
*/
$username = 'myusername';
$password = 'pass';
 
$user = new AuthenticateUser($pdo);
echo $login_status = ($user->check($username, $password)) ? 'Valid Login' : 'Invalid Login';
?>
Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532158
Share on other sites

Looks good. There's a problem with the return value of the check() method, though: If the username or password are incorrect, the method doesn't return anything (i. e. null), otherwise it returns the object instance. This violates the specification and is also very confusing. The method should return an actual boolean:

$passwordHashStmt = $this->pdo->prepare('
    SELECT password
    FROM users
    WHERE username = ?
');
$passwordHashStmt->execute([
    $username
]);
$passwordHash = $passwordHashStmt->fetchColumn();

return $passwordHash && password_verify($password, $passwordHash);

Logging out is a session feature, so it belongs into a separate Session class.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532160
Share on other sites

I was aware that it didn't return anything on false. I tried to update my post to ask you about that but the site already locked me out of editing the post.

 

The previous code comment really made sense to me when you put the task to a question, "Is the username and password valid?" which is a yes or no answer, so return Boolean. It got me to re think the process.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532161
Share on other sites

Why are you returning $passwordHash? The result is the exact same with or without it.

 

return $passwordHash && password_verify($password, $passwordHash);// result: bool true or bool false.

 

 

return password_verify($password, $passwordHash); // Same exact result: bool true or bool false.

Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532163
Share on other sites

If the user doesn't exist at all, fetchColumn() will return false. It wouldn't make sense to pass this value as a “hash” to password_verify(). To prevent this situation, there's a pre-check if $passwordHash is even valid.

 

This could also be written with an if statement, of course:

if ($passwordHash)
{
    $valid = password_verify($password, $passwordHash);
}
else
{
    $valid = false;
}

return $valid;
Edited by Jacques1
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532164
Share on other sites

The if/else is quite clear to me. The && is not. How would I see that the && translates to the same thing as the if since the results are exactly the same with or without it? I am also wondering if the if/else was better to be used for more clarity to those who may read the code.

 

We could also use a Ternary instead of the other two options as well right? (My Preference, although not as readable as the if/else IMO)

return $passwordHash ? password_verify($password, $passwordHash) : false; 
Edited by benanamen
Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532165
Share on other sites

You can use any of those. The “&&” variant takes advantage of short-circuit evaluation: If $passwordHash is false, the whole boolean operation immediately evaluates to false. The password_verify() part is completely ignored, because it's not needed to determine the result. This is a common pattern.

 

Just using password_verify() without the pre-check would be a bad idea, though, because then you rely on the function to behave “nicely” in case of invalid arguments. This is not guarenteed. You might get a warning or an exception or even trigger a bug. I wouldn't want to try that out in security-related code.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532166
Share on other sites

Before calling the current code complete, I am thinking the the DB connection code needs to be in at the least a function or a class passing the parameters to it. What would be best practice as far as the connection code. As is, if it were all in its own file complete, you have parameters that could get edited right next to code that shouldn't ever be touched. Additionally, if in a function or class, multiple connections to different DBs could be created if the need was ever there. Also, if in a class or function, should the options be hard coded or passed?

 

What is the best OOP practice for the DB connection code? I realize there is a ton of code out there, but with my lack of experience with OOP, I wouldn't know good from bad at this point.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532177
Share on other sites

For now, I'd stick to plain PDO and read the connection settings from an external configuration file while the application is initialized. You can then pass the PDO instance to the controller (or whatever part of the application is responsible for handling the current request).

 

Plain PDO is far from perfect, because it can't be used with dependency injection and makes testing very difficult. But it's the easiest solution.

Link to comment
https://forums.phpfreaks.com/topic/301020-oop-noob/#findComment-1532179
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.