Remove code duplication on lists

There is a situation when you have a list of items and you need to perform a lot of actions on them, like querying an item state or changing an item state.

In this situation, a lot of code duplication appears, more exactly a lot of foreach statements on the item list then test if it is the right item then call a method on it.

Let’s suppose that we have a User class with some command and query methods, like this:

<?php
/**
 * Copyright (c) 2017 Constantin Galbenu <gica.galbenu@gmail.com>
 */
namespace Gica;

class User
{
    private $name;
    private $active;
    private $id;

    public function __construct(
        int $id,
        string $name,
        bool $active = false
    )
    {
        $this->id = $id;
        $this->name = $name;
        $this->active = $active;
     }

    public function getId(): int
    {
        return $this->id;
    }

    public function getName(): string
    {
        return $this->name;
    }

    public function isActive(): bool
    {
        return $this->active;
    }

    public function deactivate()
    {
        $this->active = false;
    }

    public function activate()
    {
        $this->active = true;
    }
}

We also have a class that holds a list of Users; let’s name it ClassicUserList, something like this:

<?php
/**
 * Copyright (c) 2017 Constantin Galbenu <gica.galbenu@gmail.com>
 */

namespace Gica;

class ClassicUserList
{
    /**
     * @var User[]
     */
    private $users;

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

    public function activateUser(int $idUser)
    {
        foreach ($this->users as $user) {
            if ($user->getId() === $idUser) {
                $user->activate();
            }
        }
    }

    public function deactivateUser(int $idUser)
    {
        foreach ($this->users as $user) {
            if ($user->getId() === $idUser) {
                $user->deactivate();
            }
        }
    }

    public function isUserActive(int $idUser): bool
    {
        foreach ($this->users as $user) {
            if ($user->getId() === $idUser) {
                return $user->isActive();
            }
        }

        return false;/* if user does not exist*/
    }

    public function userExists(int $idUser): bool
    {
        foreach ($this->users as $user) {
            if ($user->getId() === $idUser) {
                return true;
            }
        }

        return false;/* if user does not exist*/
    }

    public function getUserName(int $idUser): string
    {
        foreach ($this->users as $user) {
            if ($user->getId() === $idUser) {
                return $user->getName();
            }
        }

        return '';/* if user does not exist*/
    }
}

Do you see how many foreach statements exist? There is a lot of code duplication and DRY screams at us!

The solution is to extract the code that repeats into a specialized method: callOnUser. Let’s create another class, name it UserList, that uses this pattern:

<?php
/**
 * Copyright (c) 2017 Constantin Galbenu <gica.galbenu@gmail.com>
 */

namespace Gica;

class UserList
{
    /**
     * @var User[]
     */
    private $users;

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

    public function activateUser(int $idUser)
    {
        $this->callOnUser($idUser, function (User $user) {
            $user->activate();
        });
    }

    public function deactivateUser(int $idUser)
    {
        $this->callOnUser($idUser, function (User $user) {
            $user->deactivate();
        });
    }

    public function isUserActive(int $idUser): bool
    {
        return $this->callOnUser($idUser, function (User $user) {
            return $user->isActive();
        }, false /* if user does not exist*/);
    }

    public function userExists(int $idUser): bool
    {
        return $this->callOnUser($idUser, function () {
            return true;
        }, false /* if user does not exist*/);
    }

    public function getUserName(int $idUser): string
    {
        return $this->callOnUser($idUser, function (User $user) {
            return $user->getName();
        }, '' /* if user does not exist*/);
    }

    private function callOnUser(int $idUser, callable $querier, $default = null)
    {
        foreach ($this->users as $user) {
            if ($user->getId() === $idUser) {
                return $querier($user);
            }
        }

        return $default;
    }
}

The callOnUser method is private, to restrict access to items other that intended by the public interface and it accepts a callback that will be called with the found item.

The method call the callback with item as argument (User in this example) and returns the result.

You could apply this pattern if there are a lot of forwarding methods to the items in the list and if the list is not indexed by the items ID.

Remove code duplication on lists