OOP - User Registration

[benanamen]

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');

[Jacques1]

• The names are very confusing. “Registration” sounds like this represents any kind of registration, not a user registration in particular. At the same time, “UserRegistration” sounds like a very generic implementation of a user registration which supports arbitrary storage engines, data structures and whatnot, but it's actually an ultra-specific representation of your current setup (MySQL, PDO, a table named users with particular columns etc.). The interface should be name UserRegistration, because that's what it is. Then the class should name indicate the implementation specifics.
• 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.
• 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.

I think the main problem right now is the lack of abstraction and modularity (which is normal for your first OOP designs). You've essentially taken the procedural code from classical PHP scripts and put it into methods. This works, but it doesn't take advantage of the strenghts of OOP.

 

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.

Edited January 21, 2017 by Jacques1

[Strider64]

There a plenty of books that just deal with design patterns, though boring can be useful.  An author like Larry Ullman a good PHP author can show you how to get going in OOP and I know Ullman will even recommend a few of these other authors of design patterns.

 

What I do with something that you are trying to implement is break it down to its simplest core. In this case you want to write a user's registration. Well that requires interactivity with a database table of some sort. The first thing you will have to do is Create the db Table, next it would be nice to be able to Read from the db Table, after that it would be nice to be able to edit certain fields so you want to be able to Update the db Table, and finally you want to give the administrator (and maybe the user themselves) the ability to Delete the record for the db Table.

 

So what do we have? We have CRUD and stick a interface on it then you get iCRUD.

<?php
namespace login_project\database;

/* The iCRUD interface.
 * The interface identifies four methods:
 * - create()
 * - read()
 * - update()
 * - delete()
 */
interface iCRUD {

  public function create($data);

  public function read();

  public function update($data);

  public function delete($id=NULL);
  
}

While this a basic interface that I am sure pales in comparison to other interfaces, the purpose of it to force you to build your class in a precise manner and you'll find that it can be used for other than an user's registration class. If you have a good IDE then you have an added bonus for it will give you hints and turn green on methods that you have correctly implemented in your class. Though like I said Larry Ullman or other authors of design patterns will be able to you better explanation than I. 

Edited January 21, 2017 by Strider64

[benanamen]

“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!

[ignace]

Here are 2 example interfaces.

 

interface UserRegistration {
public function __construct(UserManager $userManager, EventDispatcher $eventDispatcher);
public function register(array $data): User;
public function getErrors(): array;
}

interface UserManager {
public function findByEmail($email): User;
public function createUser(): User;
public function updateUser(User $user): UserManager;
public function deleteUser(User $user): UserManager;
}

 

The UserRegistration class would receive the data from the client, validate it, and throw an exception if it fails. The encountered errors can be retrieved with getErrors().

 

If the validation is successful, it creates the user through the UserManager, and insert the user with updateUser() (or addUser() if you prefer).

 

This is the most basic form of a user registration and it should not handle more.

 

Everything else would be handled through events:

- Sending welcome e-mails,

- Creating an invoice,

- etc...

 

Each of these event handlers may have their own interface as well.

[Jacques1]

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?

 

Because your interface is not generic. The documentation specifically says it's for user registration, and the register() method has parameters for personal data. None of this would work for, say, registering a company.

 

It's perfectly reasonable to have an interface  for user registrations, but then the name should say that. Alternatively, you could in fact write a super-generic interface for any kind of registration, but this will look very differently.

 

 

 

I don't know the "how" of this. When you say component, do you mean another interface or class?

 

Yes. A class, an interface, maybe a group of classes. Anything that manages the database-specific tasks.

 

 

 

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

 

If it's an associative array, yes. It could also be some other object representing just the user data, but that's probably overkill.

[ignace]

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?

Every registration is a user registration even when it's for a company. Or do your users register their company details but then don't bother creating a username and password?

[Jacques1]

Every registration is a user registration even when it's for a company.

 

I wouldn't make that assumption. And I don't think this is what benanamen meant.

Edited January 21, 2017 by Jacques1

[benanamen]

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');

Edited January 21, 2017 by benanamen

[Jacques1]

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

 

Yes, because a method named like the class is the constructor in older PHP versions and many other languages like Java.

 

Class names should also be nouns, not verbs, because they represent objects, not actions. And repeating the information from the class in a method is redundant. It's fairly obvious that BasicEmail::send() will send a basic e-mail. Using BasicEmail::sendBasicEmail() adds no clarity, just clutter.

 

• The user registration should not have an e-mail method. It may be the case that some implementations send e-mails around during the registration process, but it makes no sense to have this method as part of the external registration API. It also means somebody else (who?) is responsible for triggering the e-mails.
• “ProcessEmail” also suffers from the verb problem. Object-oriented programming is in fact about objects. In this case, you could either have a mail submission agent as an object. Or an e-mail itself.
• Like I said above, the MySQL-specific features should not be part of the registration workflow, because those aspects aren't related in any way. Generating tokens or sending e-mails has nothing to do with whether you use MySQL, MongoDB or plain files to store your data.

 

Passing objects to other objects can be implemented with depency injection.

[benanamen]

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');
        }
    }
}

[benanamen]

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"?

[Jacques1]

No. Again: Object-oriented programming is about objects. Objects are entities like a user, an e-mail or a database. They're not actions. So when your class names start with verbs, there's something wrong.

 

You should not create a new object for every tiny task. Then you essentially end up with procedural code where functions are masquerading as objects. You need identify sensible entities and represent those with objects.

 

For example, the entity responsible for sending e-mails would be a mail submission agent. So you create an interface and a class for a mail submission agent:

<?php

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);
}

<?php

class SMTPMailSubmissionAgent implements MailSubmissionAgent
{
    private $host;

    private $port;

    private $username;

    private $password;

    public function __construct($host, $port, $username, $password)
    {
        $this->host = $host;
        $this->port = $port;
        $this->username = $username;
        $this->password = $password;
    }

    public function send($from, $to, $subject, $body)
    {
        // TODO: Implement send() method with PHPMailer or any other library.
    }
}

Whenever a class needs to send e-mails, it uses some implementation of MailSubmissionAgent (like the STMP one):

class StandardUserRegistration implements UserRegistration
{
    private $mailSubmissionAgent;

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

    public function register($username, $password, $emailAddress)
    {
        // ...

        $this->mailSubmissionAgent->send('info@mysite.com', $emailAddress, 'Your registration', '...');

        // ...
    }
}

The point of having two objects is that the details of sending an e-mail are completely up to the MailSubmissionAgent and have no bearing whatsoever on the user registration. You can change the mail class, you can add new mail classes, you can delete existing ones. At no point do you have to touch the registration procedure. And that kinda makes sense.

 

The same can be done with the database. Why should the registration procedure depend on the storage system you happen to use? This is a completely unrelated task which should be managed by a separate entity.

 

It's like a company where the jobs are strictly separated. Creating the GUI is up to a designer, planning and maintaining the database is up to a DBA, writing code is up to a programmer. Everybody performs one specific task, and the exact details are their problem. The project manager doesn't care whether the programmer uses PhpStorm or Netbeans to write the code. They just want the result.

Edited January 22, 2017 by Jacques1

[benanamen]

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");
    }
}

Edited January 22, 2017 by benanamen

[Jacques1]

Yes, yes and yes.

 

I don't instantiate an interface. I call the send() method of an object, and that object is an instance of a class which implements the MailSubmissionAgent interface. For example:

<?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);

// now the registration object can use the mail object
$userRegistration->register(...);

Edited January 22, 2017 by Jacques1

[benanamen]

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');

Edited January 22, 2017 by benanamen

[ignace]

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
}

 

You will probably have to create or update users in multiple places in your application so I would introduce a (interface) UserManager (=concrete PDOUserManager) to CRUD users.

 

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.

 

interface UserMailer extends MailSubmissionAgent {
public function sendWelcomeMail(User $user);
public function sendForgotPasswordMail($email);
// you get the idea
}

Edited January 23, 2017 by ignace

[benanamen]

 

 

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

[ignace]

No. That is a general class to send e-mails. I would create a specialised interface to send user related e-mails as demonstrated in my example.

 

This way you can force the required parameters and decouple your code from knowing how to send these sort of e-mails otherwise you'd end up with code like:

 

$to = $user->toEmail();
$subject = $this->translator->translate('Hello %name%', ['%name%' => $user->getFullName()]);

$variables = [
// ..
];

$templateBody = $this->templateManager->getTemplateByName('user_registration');
$this->mailSubmissionAgent->send($to, $subject, $templateBody->resolve($variables), 'noreply@mydomain.com');

 

While this otherwise would be:

 

$this->userMailer->sendWelcomeMail($user);

Edited January 26, 2017 by ignace