HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Simple PHP Login Register Script with OOP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptsimplewithphploginregisteroop

Problem

I am relatively new to programming and OOP in PHP. I tried to create a Simple Login Register Script using my basic knowledge of OOP. I'm sure my code can be better in a lot of way. I'm trying to code better and learn new things.

Be harsh, please find out as many small noob mistakes as you can. Suggestions and Feedbacks are always welcomed !

Config.php

conn = mysqli_connect($this->host,$this->username,$this->password);

    if (!$this->conn) {
        die("Connection failed: " . mysqli_connect_error());
    }
    else{
        echo "Connected successfully to server";
    }

    $db_selected = mysqli_select_db($this->conn, $this->dab);

    if (!$db_selected) {
        // if the given database doesn't exists
        // creates new database with that name
        $db_sql = 'CREATE DATABASE chatapp';

        // verify the database is created
        if (mysqli_query($this->conn, $db_sql)){
            echo "Database chatapp already exists or created successfully\n";
        } else {
            echo 'Error creating database: ' . mysqli_error() . "\n";
        }
    }

    // creating tables
    $table_sql = "CREATE TABLE IF NOT EXISTS users (".
            "uid INT PRIMARY KEY AUTO_INCREMENT,".
            "username VARCHAR(30) UNIQUE,".
            "password VARCHAR(50),".
            "name VARCHAR(100),".
            "email VARCHAR(70) UNIQUE); ";

    // verify the table is created
        if (mysqli_query($this->conn, $table_sql)) {
            echo "Table: users already exists or created successfully\n";
        } else {
            echo 'Error creating table: ' . mysqli_error($table_sql) . "\n";
        }
}
}

$obj = new dbConfig();

$obj->host = 'localhost';
$obj->username = 'root';
$obj->password = '';
$obj->dab = 'chatapp';
$obj->dbConnect();


login.php

```
conn,$_POST['emailusername']);
$password = mysqli_real_escape_string($obj->conn,$_POST['password']);
$password = md5($password);

$sql="SELECT uid FROM users WHERE username='$emailusername' or

Solution

At first glance

After a quick glance at your code, it reminded me of this review I posted a while back. Much like that piece of code, a couple of (broadly similar) issues jumped out:

  • Wrapping procedural style code in a class with some methods does not qualify as OOP.



  • Your one and only method (dbConnect) is fundamentally flawed and unsafe. Not in the least because it relies on the user (the code that creates an instance of your class) to set the properties manually before calling the method. What if these properties aren't set? create a __construct method (constructor) that requires the user to pass the connection params to the class when an instance is created, much in the same way that mysqli or PDO require these params to be passed.



  • There is some sanitation of values used in your query, but you really ought to look into prepared statements



  • Methods don't echo, and they certainly don't die. If they encounter a problem, they throw new RuntimeException, or some other exception that accurately describes the problem (InvalidArgumentException, BadMethodCallException, ...).



  • Your dbConnect method just does too much. Its name suggests it merely connects, when in reality, it echoes, creates tables and never closes the DB connection. Calling dbConnect twice is possible, your code doesn't take that into account.



  • Coding standards are important. PHP doesn't have any official standards (yet), but the PHP-FIG standards are adopted by all major players (Symfony, Zend, Apple, Composer, CakePHP, ...). You'd do well following them, too



  • md5, as a hash, is no longer secure. It first started showing weaknesses back in 1998, was deemed unsafe about ten years ago, and back in 2010, a single-block collision was found. At the very least, use sha1, but even so: PHP supports blowfish, sha256 and sha512 just as easily.



  • When hashing passwords, you'd also do well using salt. Don't just hash the password as-is. Add some random string that only you know. Even so, PHP has a very useful, easy to use and helpful function called password_hash. Use it.



  • You seem to be confused as to the point of OOP. In PHP, and many other languages, OOP isn't used to reduce the size of your code base (look at Java, or even C++). It's about making a sizable code base more manageable, more structured. Yes, when you first start writing OO code, you'll find yourself writing a lot more code than you were used to. The benefits are that, after a while, you'll find yourself re-using bits of code that you had written for another project. OOP enable (or at least facilitates) abstraction, separation of concerns and force developers to think more about what task should go where in the application. If you want to write OO code, don't go looking for ways to reduce the amount of code you write, look for ways to improve the overall structure of your project, and optimize what component has access to what functionality at any given time.



I'll be going into all of these issues in detail, each time updating this answer, but for now, I'd urge you to check out the links I provided here, and google some OOP terms like the SOLID principles

If you want to avoid lengthy code at all costs, then perhaps you should consider learning another language than PHP. Perl, for example, allows you to write obfuscated code a lot easier. Mind you, PHP has a couple of tricks up its sleeve, as this "Hello world" demonstrates (source):

<?=${[${[${[${[${[${[${[${[${${![]}.=[]}.=${![]}{!![]}]}.=${!![${[${[${[${[${[${[${[]}++]}
++]}++]}++]}++]}++]}++]}{![]+![]+![]}]}.=${[${[${[${[]}++]}++]}++]}{![]+![]}
.${![]}{![]+![]+![]}]}.=${[${![]}=${![]}{!![]}]}{!![${!![${!![${![]}++]}++]}++]}^
${!![${[${[${[]}++]}++]}++]};


Update 1

As you requested, some more details on the second issue I mentioned (about the dbConnect method being flawed). Your code, as it stands now expects the user to write something like:

$obj = new dbConfig();
$obj->host = '127.0.0.1';
$obj->username = 'user';
$obj->password = 'pass';
$obj->dab = 'dbname';
$obj->dbConnect();


But what if, because most devs are lazy, someone writes:

$db = new DbConfig();//classes should start with an UpperCase
//some code
someFunction($db);
//where someFunction looks like:
function someFunction(DbConfig $db)
{
    $db->dbConnect();//<== ERROR! host, user, pass... nothing is set
}


The instance could be created in a different file as the line that is calling dbConnect, so debugging this kind of code is hellish. You could re-write each piece of code that calls dbConnect to make sure all of the required properties have been set correctly, but what if they aren't? And what is "correct"? That's something the instantiating code should know, not the code that calls dbConnect. Luckily, you can use a constructor to ensure all parameters are known:

```
class DbConnect
{
/**
* @var string
*/
protected $host = null;

/**
* @var string
*/
prot

Code Snippets

<?=${[${[${[${[${[${[${[${[${${![]}.=[]}.=${![]}{!![]}]}.=${!![${[${[${[${[${[${[${[]}++]}
++]}++]}++]}++]}++]}++]}{![]+![]+![]}]}.=${[${[${[${[]}++]}++]}++]}{![]+![]}
.${![]}{![]+![]+![]}]}.=${[${![]}=${![]}{!![]}]}{!![${!![${!![${![]}++]}++]}++]}^
${!![${[${[${[]}++]}++]}++]};
$obj = new dbConfig();
$obj->host = '127.0.0.1';
$obj->username = 'user';
$obj->password = 'pass';
$obj->dab = 'dbname';
$obj->dbConnect();
$db = new DbConfig();//classes should start with an UpperCase
//some code
someFunction($db);
//where someFunction looks like:
function someFunction(DbConfig $db)
{
    $db->dbConnect();//<== ERROR! host, user, pass... nothing is set
}
class DbConnect
{
    /**
     * @var string
     */
    protected $host = null;

    /**
     * @var string
     */
    protected $username = null;

    /**
     * @var string
     */
    protected $pass = null;//null for no pass

    /**
     * @var string
     */
    protected $dbName = null;

    /**
     * @var mysqli
     */
    protected $conn = null;

    public function __construct($host, $user, $pass = null, $db = null)
    {
        $this->host = $host;
        $this->user = $user;
        $this->pass = $pass
        $this->dbName = $db;
    }

}
$db = new DbConnect('127.0.0.1', 'root', '', 'chatapp');

Context

StackExchange Code Review Q#75181, answer score: 5

Revisions (0)

No revisions yet.