Creating an Entity, persisting it to the DB, and sending a welcome email

Question

The reason I'm posting this piece of code is because I want to know how I'm supposed to properly handle the change in requirements/specifications and how I should accommodate it in unit testing.

/**
 * Create an Affiliate
 */
public function createUser($username, $password, $email, $firstName, $lastName, $ip, $notifications = null) {
    // These values are assumed to be validated prior to this function being
    // called.

    $date = new \DateTime();

    $user = new Affiliate();
    $user->setUsername($username)
         ->setPassword(AffiliateUtils::hashPassword($password))
         ->setStatus(Affiliate::STATUS_PENDING)
         ->setEmail($email)
         ->setFirstName($firstName)
         ->setLastName($lastName)
         ->setLastIp($ip)
         ->setRegisteredIp($ip)
         ->setLastDate($date)
         ->setRegisteredDate($date);

    try {
        $this->em->persist($user);
        $this->em->flush();
    } catch (DBALException $e) { return false; }

    if ($notifications != null)
        $notifications->sendAffiliateEmail("WELCOME_AFFILIATE", $user);

    return true;
}

This function is apart of an AffiliateHandler, which is responsible for the various business-logic-related operations on an Affiliate which is a database entity (active record). This function is called when a user registers for my site.

Prior to the change in requirements, the $notifications parameter was not required. The notifications class is responsible for sending a welcome email containing a link for the user to verify their email. I added a default value to the variable because the creation of the user is not dependent upon a notification and I didn't want to break/modify the unit tests associated with this particular function.

I feel like this is bad style because this function is doing too many things. Also, it creates somewhat of a dependency on the notifications class. But at the same time, if I break up the function, I need to have access to the new user created so I can send him/her the notification.

What should I be doing to handle this change without adding too much complexity to my code?

Answer 1

Just some thoughts:

1) parameters instead of properties - createUser function has various user data as parameters. I think it is better to have a single object with properties instead (e.g. $userData). If information is added to the user entity, properties are added, not extra parameters in the function.

2) auto mapping - in many cases members/properties must be copied from one object to another (e.g. data coming from a form into an object to be persisted in the database). This means tedious work for the programmers, so this one reason auto mappers were invented. For PHP, Nylle is one (never used, though).

A mapper is handling copying of data from an object to another based on property names, by default (a case of convention over configuration).

For your particular case, assuming that user data comes in a $userData instance and that it has the same properties, it should look like this:

$user = $mapper->Map(new \Affiliate(), $userData);

Of course, for several properties, it is overkill (and also the configuration is more repugnant that the bigger brother's one - .NET Automapper), but may be useful when dealing with more.

3) Separation of concerns - you can (should) separate notification sending from user creation, as they different things and also automatic tests should treat them differently (e.g. you can automatically create test users on some test database, but surely you do not want to send lots of test e-mails / mock e-mail sending):

• createUser may return the newly created user
• another function may use this instance to send the e-mail

4) Error logging - Db persist failure is handled only by returning false to the caller. It is recommended to log these failures using a logging framework or a custom one.