Refactoring Patterns #2 Extract Method

Refactoring Patterns #2 - Extract Method

Mit dem Extract Method Pattern kann eine Methoden in einer Klasse verkürzt werden. Hierbei wird ein Teil des Codes in eine neue Methode ausgelagert.
 

Als Beispiel dafür dient die Klasse "UserRegistrationService". In der Methode "registerUser" wird zunächst geprüft, ob der gleiche Benutzer bereits existiert, ob das Passwort1 und Passwort2 identisch ist und ob das Passwort lang genug ist. In einer realen Implementierung gibt es bestimmt noch weitere checks und würde man statt mysqli vielleicht auch ein Framework verweden.


Der gezeigt PHP-Code dienst also nur zur Demonstation:

class UserRegistrationService{

  /**
   * Saves a user registation to the database when the user does not exists and the passwords
   * meet several validation contraints.
   *
   *
   * @param string $username
   * @param string $password1
   * @param string $password2
   *
   * @throws NoDatabaseConnectionException
   * @throws UserAllreadyExistsException
   * @throws PasswordsDoNotMatchException
   * @throws PasswordToShortException
   *
   * @return boolean
   */
   public function saveRegistration($username, $password1, $password2)
   {
        // count existing usersI
        $conn = new mysqli('localhost', 'dbuser', 'dbpassword', 'users');
        if ($conn->connect_error) {
                throw new NoDatabaseConnectionException("Connection failed: " . $conn->connect_error);
        }

        $query = "select * from users where username='" . $conn->real_escape_string($username) . "'";
        $result = $conn->query($query);
        $existingUserCount = $result->num_rows;
        if ($existingUserCount > 0) {
                throw new UserAllreadyExistsException("The user allready exists");
        }

        if ($password1 !== $password2) {
                throw new PasswordsDoNotMatchException("Password1 and Password2 are not same");
        }


        if (strlen($password1) < 8) {
                throw new PasswordToShortException("The password was to short");
        }

        // user is valid
        $escapedUser = $conn->real_escape_string($username);
        $escapedPassword = hash('sha512', $conn->real_escape_string($password1));
        $query = "insert into users (username, password) values ('" . $escapedUser  
           ."', '" . $escapedPassword . "');";

        $result = $conn->query($query);
        if($result !== true) {
                throw new Exception("Error during user creation");
        }

        return true;
    }
}

Die Implementierung der Methode "saveRegistration" hat über 30 Zeilen. Es ist schwer zu verstehen was darin passiert und es passieren mehrere Dinge hintereinander.

     

 

Code mit dem Extract Method Pattern in eigene Methoden auslagern

Mit dem "Extract Method"-Pattern können Teile in einer Methode ausgelagert werden, die von anderen Teilen unabhängig sind. Ein erster Schritt könnte also sein, den Teil auszulagern, der prüft, ob es schon einen Benutzer gibt. Ein bisschen tricky ist dabei, dass die Datenbankverbindung auch später genutzt wird, um den Benutzer zu speichern. Aus diesem Grund extrahieren wird gleich zwei Methoden:

  1. connectToDatabase()
  2. getUserExists($username)
class UserRegistrationService{

   protected $connection;

   /**
   * Creates a database connection.
   */
   protected function connectToDatabase()
   {
        $this->connection = new mysqli('localhost', 'dbuser', 'dbpassword', 'users');
        if ($this->connection->connect_error) {
                throw new NoDatabaseConnectionException("Connection failed: " . $conn->connect_error);
        }
   }

   /**
   * Checks if the user allready exists.
   *
   * @param string $username
   * @return bool
   */
   protected function getUserExists($username)
   {
        $this->connectToDatabase();
        $query = "select * from users where username='" . $this->connection->real_escape_string($username) . "'";
        $result = $this->connection->query($query);
        return $result->num_rows > 0;
   }

  /**
   * Saves a user registation to the database when the user does not exists and the passwords
   * meet several validation contraints.
   *
   *
   * @param string $username
   * @param string $password1
   * @param string $password2
   *
   * @throws NoDatabaseConnectionException
   * @throws UserAllreadyExistsException
   * @throws PasswordsDoNotMatchException
   * @throws PasswordToShortException
   *
   * @return boolean
   */
   public function saveRegistration($username, $password1, $password2)
   {
        if ($this->getUserExists($username)) {
                throw new UserAllreadyExistsException("The user allready exists");
        }

        if ($password1 !== $password2) {
                throw new PasswordsDoNotMatchException("Password1 and Password2 are not same");
        }


        if (strlen($password1) < 8) {
                throw new PasswordToShortException("The password was to short");
        }

        // user is valid
        $this->connectToDatabase();
        $escapedUser = $this->connection->real_escape_string($username);
        $escapedPassword = hash('sha512', $this->connection->real_escape_string($password1));
        $query = "insert into users (username, password) values ('" . $escapedUser  ."', '" . $escapedPassword . "');";

        $result = $this->connection->query($query);
        if($result !== true) {
                throw new Exception("Error during user creation");
        }

        return true;
    }
}

Die Implementierung der Methode "saveRegistration" ist nun schon deutlich kürzer. Aber nach dem ersten Refaktoring können weitere folgen. Der Code in der Methode enthält nun im wesentlichen noch zwei Teile.

  1. Validierung der Eingabedaten (Existiert der Benutzer und stimmen Passwortlänge, ...) => Zeile 48-59
  2. Speichern der Daten, wenn alles korrekt ist. => Zeile 61 - 70

Diese beiden Teile lassen sich nun ebenfalls in eigene Methoden extrahieren:

  1. validateUser($username, $password1, $password2)
  2. storeUser($username, $password)

 

class UserRegistrationService{

   protected $connection;

   /**
   * Creates a database connection.
   */
   protected function connectToDatabase()
   {
        $this->connection = new mysqli('localhost', 'dbuser', 'dbpassword', 'users');
        if ($this->connection->connect_error) {
                throw new NoDatabaseConnectionException("Connection failed: " . $conn->connect_error);
        }
   }

   /**
   * Checks if the user allready exists.
   *
   * @param string $username
   * @return bool
   */
   protected function getUserExists($username) 
   {
	$this->connectToDatabase();
        $query = "select * from users where username='" . $this->connection->real_escape_string($username) . "'";
        $result = $this->connection->query($query);
        return $result->num_rows > 0;  
   }

   /**
   * Stores a user in the database.
   *
   * @param string $username
   * @param string $password
   *
   * @return bool
   */
   protected function storeUser($username, $password)
   {
        $this->connectToDatabase();
        $escapedUser = $this->connection->real_escape_string($username);
        $escapedPassword = hash('sha512', $this->connection->real_escape_string($password));
        $query = "insert into users (username, password) values ('" . $escapedUser  ."', '" . $escapedPassword . "');";

        $result = $this->connection->query($query);
        if($result !== true) {
                throw new Exception("Error during user creation");
        }

	return true;
   }

   /**
   * Validates the user data and throws an exception in the case when the user data is invalid
   *
   * @param string $username
   * @param string $password1
   * @param string $password2
   */  
   protected function validateUser($username, $password1, $password2)
   {
        if ($this->getUserExists($username)) {
                throw new UserAllreadyExistsException("The user allready exists");
        }

        if ($password1 !== $password2) {
                throw new PasswordsDoNotMatchException("Password1 and Password2 are not same");
        }

        if (strlen($password1) < 8) {
                throw new PasswordToShortException("The password was to short");
        }
   }

   /**
   * Saves a user registation to the database when the user does not exists and the passwords
   * meet several validation contraints.
   *
   *
   * @param string $username
   * @param string $password1
   * @param string $password2
   *
   * @throws NoDatabaseConnectionException
   * @throws UserAllreadyExistsException
   * @throws PasswordsDoNotMatchException
   * @throws PasswordToShortException
   *
   * @return boolean
   */
   public function saveRegistration($username, $password1, $password2)
   {
	$this->validateUser($username, $password1, $password2);

	// user is valid, we can store it
	return $this->storeUser($username, $password1);
    }
}

Woran Code für Extract Method erkennen

Klassischerweise kann man in langen Methoden nach Stellen suchen, die auf den selben Variaben arbeiten. Möglicherweise wird nur mit dem Ergebnis einer Variable im Code weitergearbeitet? Wenn das der Fall ist, sind solche Blöcke ein Kandidat für "Extract Method".

Fazit

Der Code der Klasse ist ingesamt länger geworden, aber dafür ist die Logik in den einzelnen Methoden besser zu verstehen und gekapselt. Die längste Methode hat gerade 10 Zeilen.

Durch die Anwendung des "Extract Method"-Patterns ergeben sich ggf. weitere Möglichkeiten. Zum Beispiel kann man erkennen, dass manche Methoden nur Datenbankspezifisch sind (z.b. connectToDatabase, getUserExists, oder storeUser). In einem weiteren Schritt könnten diese Methoden mit dem "Extract Class"-Pattern in eine eigene Klasse extrahiert werden.
 

Navigation