[Checking if a checkbox is checked inside a foreach loop]

You are using obsolete code that has been removed from Php. You need to use PDO with prepared statements. You are also vulnerable to an SQL Injection Attack. You never ever use variables in your query. The entire thing needs to be tossed.

 

https://phpdelusions.net/pdo

[Need help please]

Look at your first if. Your POST is wrong. $POST should be $_POST

[OOP - Database Interface]

What I fail to see is the OO part. So far, your objects are just containers for functions. They have no attributes to hold their state, they have no task which they take care of autonomously.

 

If you don't see it, it is because I don't know to do it, otherwise it would be there. So perhaps that is actually the next part I need to learn about. I don't know about attributes holding state in OOP. I don't know about autonomously taking care of tasks in OOP. What can you tell me about that in a way I can understand?

 

At this point, I don't know enough to make decisions of how I would personally want to implement anything in OOP. I am looking for the definitive way to do things for as much as that is possible. One of the things I don't like about Php is there is just way too many ways to do the same exact thing. One of the things I liked about implementing interfaces is it is definitive in that if you don't use the methods, it doesn't work. It is a strict rule.

[OOP - Database Interface]

I think you are not understanding my approach. I know there is much more to be done. I am basically taking this a piece at a time to make sure I am getting that piece correct and trying to avoid information overload. What you see "left over" in procedural is what are the next steps to understand in OOP correctly. For a DB, making a connection is step one. Now obviously I need to learn the proper way to handle CRUD. When I ask you questions, it is not to question what you tell me, it is to understand the "why" and not just to know that's what I should do. e.g I know to turn on a light you flip the switch and the light comes on, but I want to know "why" it does. You can't really be a Master of something if you don't know it inside and out.

 

It's not procedural code with some classes.

 

That is not my intent despite what the readme on my repo says. There is a particular reason it was worded like that. My intent is to re-write that project in complete, optimum OOP best practices. I have read numerous car, bicycle, animal, and people OOP tutorials and am familiar with what they all said. How much is correct or could be better, I have no idea as of now.

 

So, you know my project, you know what my intent is, and you can see the code at my repo that I am transforming to OOP. If you're willing, that should make it easy to guide me, I am more than willing to put in the time to learn and understand what you tell me. So, yes, I do want to learn OOP, but I want to learn it correctly from the start.

 

Moving forward, it seems my next steps are to do something with the CRUD operations. And yes, I have read many a code that others have done which just appears they are probably doing it wrong since none of them do it near the same way.

 

My question is, would the CRUD operation for a particular DB type (e.g Mysql) go in the class that implements the connection or is there some super crud class design that any type of db can use?

 

Additionally, since you could be using two DB's at once (Mysql & MongDB), how should the actual credentials be set up?

 

Thanks to all who help!

[Split an array element and place it back in array]

Your code is vulnerable to an SQL Injection Attack. You never ever put variables in a query. You need to use prepared statements. It is also a security problem outputting DB errors to the user. That info is only good to a programmer or a hacker. I suspect you have many other problems as well.

[Split an array element and place it back in array]

Where is the array coming from? A Database? How is the data getting like that in the first place?

[OOP - Database Interface]

(Data Mapper, Active Record)

 

Looking into it, that looks like what I needed to know about. I see Active Record is a pattern as well as an Open Source ORM library. Which one are you referring to? Is the pattern something I should learn how to write myself or do I just use the library?

 

This works exactly as expected. Any issues with it? Since this works, why would I need a Data Mapper or Active Record mapper?

<?php
interface Database
{
    public function connect();
}

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

class Mysql implements Database
{
    public function connect()
    {
    $dsn = DB_TYPE . ":host=" . DB_HOST . ";dbname=" . DB_NAME . ";charset=" . DB_CHARSET;
    $opt = [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
        PDO::ATTR_EMULATE_PREPARES => false,
        ];

    return new PDO($dsn, DB_USER, DB_PASSWORD, $opt);
    }
}

//----------------------------------------------------------------------------------------
// Testing
//----------------------------------------------------------------------------------------

$db = new Mysql();
$pdo = $db->connect();

$sql = 'SELECT login_username FROM user_login';
$stmt = $pdo->prepare($sql);

foreach ($pdo->query($sql) as $row)
    {
    echo $row['login_username'];
    }

[OOP - Database Interface]

The short answer to what I am doing is converting the procedural code from my repo to OOP for the sole purpose of learning advanced OOP. For as much as is possible, I want to write the "perfect app" in OOP.

[OOP - Database Interface]

As mentioned in another thread, all the OOP questions are for learning the proper way to do OOP.  The test project only consists of registration/login/forgot password/password reset functionality so that would tell you what all my questions will relate to. When I post something that doesn't make sense in relation to that functionality it is because that is the best I currently understand to do in OOP so knowing what the "project" consists of should make it easy for someone to guide me and point me in the proper direction.

 

I could read about how to do OOP all day on the net but I don't know enough yet to know if the author knows what they are talking about so I look to the couple experts on this forum that I trust and know have the proper knowledge. When I see certain procedural code I can spot in an instant if the author knows anything. i.e vars in a query, mysql_* functions. In OOP I currently can only spot if they use var or it looks like the class does a lot of things.

 

What did you actually want to do?

 

So to answer your question (which knowing the project should make obvious)

 

1. I want a person to be able to register for the app

2. Validate their email address from a secure token

3. Be able to login and logout

4. Submit a "forgot password" request

5. Reset the password from a token emailed

 

So this deals with a database, authentication, sessions etc, the usual stuff you already know. I have done this hundreds of times in procedural. 

 

For the OOP version, it should be easily scalable to other implementations such as different DB's, different logging mechanisms, etc, etc.

 

What you taught me about interfaces was a HUGE bump in the right direction. I would have been flopping around with abstract classes or something less important if it wasn't for you.

 

 

[OOP - Database Interface]

To each his own. If your code makes you happy then it's perfect already! 

 

That's your "Expert" advice to someone trying to learn? Why even bother responding with such a worthless post?

[OOP - Database Interface]

Are my questions getting too advanced for this forum now?

[how to rename files in one folder - with commandline]

Command line of what? Are you on Windows or Linux?

[OOP - Database Interface]

Based on what I have learned from this thread this is how I am thinking a database interface should be done. Do I have this right?

 

 

 

[$_REQUEST error when empty.]

You don't seem to realize what your real problem is. This is a classic XY Problem. See my signature for an explanation. You either want to do it right or you don't. Your code, including what you didnt post is junk and not fixable. It requires a complete rewrite. If you just want it to "work" until you're on a server with current Php and it just plain won't work no matter what, then someone else can "help" you. What happened to "open to new ways"?

[$_REQUEST error when empty.]

Okay, for starters, study this PDO tutorial and let me know when you have a grasp of it and we will go from there. The very first thing you're going to do is use PDO instead of the obsolete mysql_* code.

 

https://phpdelusions.net/pdo

 

You are also going to stop using REQUEST and your going to stop echoing HTML. 

 

It would be a good idea to post an sql dump of your DB if you are able. You can also PM it to me.

1. I need to make sure it is correct

2. I can follow along with something I can run

 

* While not a problem, an id column is named id or id_somthingrelated. iid is a bit odd. (Yeah, i get that it probably stands for inventory id.)

[$_REQUEST error when empty.]

What nobody has mentioned is that your code is obsolete and insecure and has been completely removed from Php. If you want to do this correctly and have the freedom to change code as required we can talk. If you are of the mindset that you just want what you have to "work", or "have to" use this code then you are on your own as far as I go. The code is just all kinds of wrong.

[OOP - User Registration]

 

 

I would also create a separate class to send the registration e-mail perhaps one class to send all sort of user related e-mails.

 

That is already in the code posted.

 

class PHPMailSubmissionAgent implements MailSubmissionAgent

[PHP read/Write]

Write a coherent question and you might get some help.

[OOP - User Registration]

I have integrated and tested the mailing Interface and all works as expect. If there is no further changes needed I would like to move on to the following that I am not for sure where to begin. I assume it is something similar to the other Interfaces.

 

the exact storage implementation has nothing to do with the registration workflow, so it should be put into a separate database-specific component.

 

I am guessing this is what you mean. Am I on the right track?

<?php
interface Database 
{
public function Connection();
}

class ConnectMysql implements Database
{
public function Connection()
    {
    $dsn = DB_TYPE . ":host=" . DB_HOST . ";dbname=" . DB_NAME . ";charset=" . DB_CHARSET;
    $opt = [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
        PDO::ATTR_EMULATE_PREPARES => false,
        ];
try
    {
    $pdo = new PDO($dsn, DB_USER, DB_PASSWORD, $opt);
    }
catch(PDOException $e)
    {
    $error = $e->getMessage() . ' in ' . $e->getFile() . ' on line ' . $e->getLine();
    error_log(MYSQL_DATETIME_TODAY . "|$error\r\n", 3, ERROR_LOG_PATH);
    }
}

RegisterUser.php (Current Working)

<?php
/**
 * Interface MailSubmissionAgent
 */
interface MailSubmissionAgent
{
    /**
     * Sends an e-mail to a single address
     *
     * @param $from    string the sender address
     * @param $to      string the receiver address
     * @param $subject string the mail subject
     * @param $body    string the mail body
     */
    public function send($from, $to, $subject, $body);
}

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

class PHPMailSubmissionAgent implements MailSubmissionAgent

{
    /**
     * @param string $to
     * @param string $subject
     * @param string $body
     * @param string $from
     */
    public function send($to, $subject, $body, $from)
    {
        mail($to, $subject, $body, "From: $from");
    }
}

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

interface UserRegistration
{
    /**
     * @param $to
     * @param $username
     * @param $password
     * @return mixed
     */

    public function register($to, $username, $password);
}

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

class StandardUserRegistration implements UserRegistration

{
    /**
     * @var PDO the connection to the underlying database
     */
    protected $database;

    /**
     * @var MailSubmissionAgent
     */
    private $mailSubmissionAgent;


    /**
     * StandardUserRegistration constructor.
     * @param MailSubmissionAgent $mailSubmissionAgent
     * @param PDO $database
     */
    public function __construct(MailSubmissionAgent $mailSubmissionAgent, PDO $database)
    {
        $this->mailSubmissionAgent = $mailSubmissionAgent;
        $this->database = $database;
    }

    /**
     * @param string $to
     * @param string $username
     * @param $password
     * @return mixed|void
     * @throws Exception
     */
    public function register($to, $username, $password)
    {
        $raw_token = mcrypt_create_iv(16, MCRYPT_DEV_URANDOM);
        $encoded_token = bin2hex($raw_token);
        $token_hash = hash('sha256', $raw_token);
        $hashed_password = password_hash($password, PASSWORD_DEFAULT);
        try
        {
            $sql = '
            INSERT INTO
              users (email, username, password, confirmation_key)
            VALUES (?, ?, ?, ?)';
            $stmt = $this->database->prepare($sql);
            $stmt->execute([$to, $username, $hashed_password, $token_hash]);
            $subject = 'Confirm Email';
            $body = "Click to activate account\r\n" . APPLICATION_URL . "/activate.php?k=$encoded_token";
            $this->mailSubmissionAgent->send($to, $subject, $body, ADMIN_EMAIL_FROM);
            die(header("Location: ./login.php?confirm"));
        }

        catch(PDOException $e)
        {
            if ($e->getCode() == '23000')
            {
                $error[] = "Registration Failed";
                $error[] = "Invalid Username or Email";
                show_form_errors($error);
            }
            else
            {
                throw new Exception($e);
            }
        } // End Catch
    }
}

// ----------------------------------------------------------------------------------------
// TESTING
// ----------------------------------------------------------------------------------------

require ('../config.php');

// create an instance of a class which implement MailSubmissionAgent
$mailSubmissionAgent = new PHPMailSubmissionAgent();

// create a registration object, passing the mail instance to the constructor
$userRegistration = new StandardUserRegistration($mailSubmissionAgent, $pdo);

// now the registration object can use the mail object
$userRegistration->register('user@example.com', 'username', 'password');

[OOP - User Registration]

I know you can't call an Interface directly (Instantiate). In the following code you call the interface directly. Is this "Dependency Injection"? And is the text MailSubmissionAgent type hinting?

    public function __construct(MailSubmissionAgent $mailSubmissionAgent)
    {
        $this->mailSubmissionAgent = $mailSubmissionAgent;
    }

Going with your interface MailSubmissionAgent would this be a correct alternative implementation of the interface?

class PHPMailSubmissionAgent implements MailSubmissionAgent
{
    public function send($from, $to, $subject, $body)
    {
        mail($to, $subject, $body, "From: $from");
    }
}

[OOP - User Registration]

After some digging, isn't my previous post referring to SOLID and the mantra "A class should only do ONE thing and do it really well"?

[OOP - User Registration]

Are we talking about a flow like this where each part (validate fields, insert record, generate token/insert token, email token) is completely a separate interface/class  and knows nothing whatsoever of anyone else?

 

(Quick throw together)

<?php
if ($_SERVER['REQUEST_METHOD'] == 'POST'){

$validate = new ValidateRegistration();
if ($validate->validate === false)
    {
    // Show errors
    }
else
    {
    // Attempt insert
    $register = new MysqlUserRegistration($pdo);
    if ($register->register === true)
        {

        $genToken = new TokenGenerator();
        $token = $genToken->generateToken;

        $email = new SendBasicEmail();
        $email->send('smith@example.com', 'Success', 'Registered. Your token is $token', 'from@example.com');
        }
    }
}

[OOP - User Registration]

method has parameters for personal data.

 

In reality I would never put the personal data in a users table, that would go in a person table. I was just trying to keep this simple while I am learning.

 

I would like to tackle one problem at a time, emailing token on successful registration.

 

Updated attempt:

Renamed registration interface

Removed personal information (first/last name)

Created new email interface. Removed old send email function call

Renamed registration class to a specific type name (Mysql)

 

Question: Is there any issue naming a method the same name as the class? (class SendBasicEmail, function sendBasicEmail)

 

Where I am stuck is integrating the additional emailing interface into the registration class. (Assuming I am heading the right direction.) Am I now forced to use namespaces and keyword 'use'? 

 

interface UserRegistration

<?php
/**
 * Register new user
 */
interface UserRegistration
{
    /**
     * Insert user registration data
     * @param string $email Unique email
     * @param string $username Unique Username
     * @param $password
     * @return
     * @internal param mixed $hashed_password Hashed Password
     * @internal param mixed $token_hash Hashed token
     */
    public function register($email, $username, $password);


    /**
     * @param $to
     * @param $from
     * @param $subject
     * @param $message
     * @return mixed
     */
    public function sendBasicEmail($to, $from, $subject, $message);
}

class MysqlUserRegistration

<?php
class MysqlUserRegistration implements UserRegistration
{
    /**
     * @var PDO the connection to the underlying database
     */
    protected $database;

    /**
     * Connection to underlying database
     * @param PDO $database
     */
    public function __construct(PDO $database)
    {
        $this->database = $database;
    }

    /**
     * @param string $email
     * @param string $username
     * @param $password
     * @throws Exception
     */
    public function register($email, $username, $password)
    {
        $raw_token = mcrypt_create_iv(16, MCRYPT_DEV_URANDOM);
        $encoded_token = bin2hex($raw_token);
        $token_hash = hash('sha256', $raw_token);

        $hashed_password = password_hash($password, PASSWORD_DEFAULT);

        try
        {
            $sql = '
            INSERT INTO
              users (email, username, password, confirmation_key)
            VALUES (?, ?, ?, ?)';

            $stmt = $this->database->prepare($sql);
            $stmt->execute([$email, $username, $hashed_password, $token_hash]);

            $subject = 'Confirm Email';
            $message = "Click to activate account\r\n" . APPLICATION_URL . "/activate.php?k=$encoded_token";
            sendBasicEmail($email, $subject, $message);// Problem - From interface ProcessEmail

            die(header("Location: ./login.php?confirm"));
        }
        catch (PDOException $e)
        {
            if ($e->getCode() == '23000')
            {
                $error[] = "Registration Failed";
                $error[] = "Invalid Username or Email";
                show_form_errors($error);
            }
            else
            {
                throw new Exception($e);
            }
        } // End Catch
    }
}

interface ProcessEmail

<?php
/** Process Email */

interface ProcessEmail
{
/**
* SendEmail constructor.
* @param $to
* @param $from
* @param $subject
* @param $message
*/
function sendBasicEmail($to, $from, $subject, $message);
}

class SendBasicEmail

class SendBasicEmail implements ProcessEmail
{
    function sendBasicEmail($to, $from, $subject, $message)
    {
        mail($to, $subject, $message, "From: $from");
    }
}

Testing

require('../config.php');
$test = new MysqlUserRegistration($pdo);
$test->register('smith@example.com', 'myusername', 'mypassword');

[OOP - User Registration]

“Registration” sounds like this represents any kind of registration, not a user registration in particular.

 

Exactly. I first named it UserRegistration and then thought about what you had said before about the interface not being a specific implementation. Such as, this could be a user reg, a customer reg, a vendor reg, a company reg...etc which would be handled by the classes that implement it. 

 

e.g

class UserReg implements Registration

class CustomerReg implements Registration

class VendorReg implements Registration

class CompanyReg implements Registration

 

Why is the thinking different in this case?

 

The purpose of the interface/class is too broad and too unspecific. It performs all kinds of unrelated actions like generating tokens, storing the data, sending out e-mails etc. If just one of those components change, you have to rewrite the whole class. At the same time, the method does not, for example, do any kind of validation, which is appearently supposed to happen somewhere else. You should split the vague task of a user registration into smaller, specific tasks. For example, the exact storage implementation has nothing to do with the registration workflow, so it should be put into a separate database-specific component.

 

I don't know the "how" of this. When you say component, do you mean another interface or class? I was aware something would need to be done with at least the emailing and error handling. I did what my current understanding allowed me to do.

 

The parameters of register() are too specialized. What if you want to store additonal data like the user location? Then you have to change the whole interface as well as every single implementing class. You can avoid this if you use more generic parameters like $username, $password and $additional_data.

 

Is $additional_data somehow able to represent more than just one piece of additional data?

 

You've essentially taken the procedural code from classical PHP scripts and put it into methods.

 

That is exactly what I did since I do not know what to do yet.

 

 It might make sense to start with smaller tasks. Building an entire OOP web application from scratch is tough and requires a deep understanding of design patterns as well as a lot of experience. It's not something you would do as your very first project.

 

This project is specifically for learning the optimum OOP. The scope is currently limited to Registration, Login, Forgot Password, and Password Reset. 

 

From here I would need to see examples. I don't know how implement the things you mention. I can generally understand code much, much better than explanations. Thank you for your guidance!

[OOP - User Registration]

The following is my working attempt at a user registration Interface. I am looking for feedback of what may be done wrong, documentation problems/improvements and any naming improvements.

<?php
/**
 * Register new user
 */
interface Registration
{
    /**
     * Insert user registration data
     * @param string $first_name User first name
     * @param string $last_name User last name
     * @param string $email Unique email
     * @param string $username Unique Username
     * @param $password
     * @return
     * @internal param mixed $hashed_password Hashed Password
     * @internal param mixed $token_hash Hashed token
     */
    function register($first_name, $last_name, $email, $username, $password);
}

/*****************************************************************************************/
/*****************************************************************************************/

class UserRegistration implements Registration
{
    /**
     * @var PDO the connection to the underlying database
     */
    protected $database;

    /**
     * Connection to underlying database
     * @param PDO $database
     */
    public function __construct(PDO $database)
    {
        $this->database = $database;
    }

    public function register($first_name, $last_name, $email, $username, $password)
    {
        $raw_token = mcrypt_create_iv(16, MCRYPT_DEV_URANDOM);
        $encoded_token = bin2hex($raw_token);
        $token_hash = hash('sha256', $raw_token);

        $hashed_password = password_hash($password, PASSWORD_DEFAULT);

        try
        {
            $sql = '
            INSERT INTO
              users (first_name, last_name, email, username, password, confirmation_key)
            VALUES (?, ?, ?, ?, ?, ?)';

            $stmt = $this->database->prepare($sql);
            $stmt->execute([
                    $first_name,
                    $last_name,
                    $email,
                    $username,
                    $hashed_password,
                    $token_hash]
            );

            $subject = 'Confirm Email';
            $message = "Click to activate account\r\n" . APPLICATION_URL . "/activate.php?k=$encoded_token";
            send_user_email($email, $subject, $message);

            die(header("Location: ./login.php?confirm"));
        }
        catch(PDOException $e)
        {
            if ($e->getCode() == '23000')
            {
                $error[] = "Registration Failed";
                $error[] = "Invalid Username or Email";
                show_form_errors($error);
            }
            else
            {
                throw new Exception($e);
            }
        } // End Catch
    }
}

/*****************************************************************************************/
/* TEST
/*****************************************************************************************/
require('../config.php');
$test = new UserRegistration($pdo);
$test->register('Sam', 'Smith', 'smith@example.com', 'myusername', 'mypassword');