Handling library entity

Question

Below you'll find a Symfony2 service I wrote to assist in handling the Library entity. Each user in my app has a personal library where one can upload items to and a purchased library, that contains items one has purchased. Both libraries fall under a parent library.

At several points in my controller, I need to get the library for the user to save data. If the library doesn't exist yet, I want to create it.

The two switches in the getUserLibrary() function especially bother me.

<?php

namespace Smoovi\SmooviBundle\Entity\Manager;

use Doctrine\ORM\EntityManager;

use Smoovi\SmooviBundle\Entity\Library,
    Smoovi\UserBundle\Entity\User;

class LibraryManager
{
    const TYPE_PURCHASED = 1;
    const TYPE_PERSONAL  = 2;

    protected $em;

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

    /**
     * Get a user's personal library
     *
     * @param \Smoovi\UserBundle\Entity\User $user
     * @return Library
     */
    protected function getPersonalLibrary($user)
    {
        return $this->getUserLibrary($user, self::TYPE_PERSONAL);
    }

    /**
     * Get a user's library for purchased objects
     *
     * @param \Smoovi\UserBundle\Entity\User $user
     * @return Library
     */
    public function getPurchasedLibrary($user)
    {
        return $this->getUserLibrary($user, self::TYPE_PURCHASED);
    }

    /**
     * Get or create a library based on $type
     *
     * @param User $user
     * @param $type
     * @return Library
     * @throws \Exception
     */
    protected function getUserLibrary(User $user, $type)
    {
        $libRepo = $this->em->getRepository('SmooviSmooviBundle:Library');

        // we first try to find the user's parent library
        // if not, create it
        /** @var \Smoovi\SmooviBundle\Entity\Library $topLibrary */
        $topLibrary = $libRepo->findOneBy(array('user' => $user, 'parent' => null));
        if (is_null($topLibrary)) {
            $library = new Library();
            $library
                ->setUser($user)
                ->setName($user->getFirstname()."'s Library")
                ->setIndex(1)
            ;
            $this->em->persist($library);
        }

        // look for the sublibrary
        // create it if necessary
        $where = array('user' => $user, 'parent' => $topLibrary);
        switch($type) {
            case self::TYPE_PURCHASED:
                $where['purchased'] = true;
                $libraryName = 'Purchases';
            break;
            case self::TYPE_PERSONAL:
                $where['personal'] = true;
                $libraryName = 'Objects';
            break;
            default:
                throw new \Exception('Library subtype not implemented');
        }
        /** @var \Smoovi\SmooviBundle\Entity\Library $subLibrary */
        $subLibrary = $libRepo->findOneBy($where);
        if (is_null($subLibrary)) {
            $subLibrary = new Library();
            $subLibrary
                ->setParent($topLibrary)
                ->setUser($user)
                ->setName($libraryName)
                ->setIndex(1)
            ;
            switch($type) {
                case self::TYPE_PURCHASED:
                    $subLibrary->setPurchased(true);
                break;
                case self::TYPE_PERSONAL:
                    $subLibrary->setPersonal(true);
                break;
                default:
                    throw new \Exception('Library subtype not implemented');
            }
            $this->em->persist($subLibrary);
        }
        $this->em->flush();

        return $subLibrary;
    }
}

Answer 1

Switch statements can often be a sign that you need an extended class. Doctrine supports Inheritance Mapping. With some liberal re-factoring I created a PersonalLibrary and PurchasedLibrary, each which extend "Library". The various classes might look like this:

/**
 * @Entity(repositoryClass="LibraryRepository")
 * @InheritanceType("SINGLE_TABLE")
 * @DiscriminatorColumn(name="libraryType", type="string")
 * @DiscriminatorMap({1 = "PurchasedLibrary", 2 = "PersonalLibrary"})
 */
class Library
{
    //abbreviated class. add the properties and setters/getters as needed

    /**
     * @param \Smoovi\UserBundle\Entity\User $user
     * @param null|Library $parent
     */
    public function __construct(User $user, Library $parent = null)
    {
        $this->setUser($user);
        $this->setParent($parent);
        $this->setIndex(1);
    }

    public function setUser(User $user)
    {
        $this->user = $user;
        $this->setName($user->getFirstname()."'s Library");
    }
}

class PurchasedLibrary extends Library
{
    public function __construct(User $user, Library $parent = null)
    {
        parent::__construct($user, $parent);
        $this->setLibraryType(1);
    }

    public function setUser(User $user)
    {
        $this->setUser($user);
        $this->setName('Objects');
    }
}

class PersonalLibrary extends Library
{
    public function __construct(User $user, Library $parent = null)
    {
        parent::__construct($user, $parent);
        $this->setLibraryType(2);
    }

    public function setUser(User $user)
    {
        $this->setUser($user);
        $this->setName('Purchases');
    }
}

class LibraryRepository extends EntityRepository
{
    /**
     * Get a user's library. If unable to create, return null.
     *
     * @param \Smoovi\UserBundle\Entity\User $user
     * @param int $type
     * @return null\Library
     */
    public function getOrCreateLibrary(User $user, $type)
    {
        $parent = $this->getOrCreateParentLibrary($user);

        $sub = $this->findOneBy(array(
            'user' => $user,
            'parent' => $parent,
            'libraryType' => $type
        ));
        if (! $sub) {
            $sub = $this->createNewLibrary($user, $type, $parent);
            if (! $sub) {
                return null;
            }
        }

        $this->persist($sub);

        return $sub;
    }

    public function createNewLibrary(User $user, $type, $parent = null)
    {
        switch ($type) {
            case 1:
                return new PersonaLibrary($user, $parent);
                break;
            case 2:
                return new PurchasedLibrary($user, $parent);
                break;
        }

        return null;
    }

    private function getOrCreateParentLibrary(User $user)
    {
        $parent = $this->findOneBy(array(
            'user' => $user,
            'parent' => null,
            'libraryType' => null,
        ));
        if (! $parent) {
            $parent = new Library($user);
        }

        return $parent;
    }
}

Notes:

• I'm using a Library Repository instead of a Library Manager. It's how I'd do it but, your LibraryManager may be just fine too.
• I moved much of the behavior (deriving name, etc) into the setters of the Entities. This free's your Manager/Repository from this concern.
• I added user and parent to the base Library Entity constructor. I'm assuming that in your domain, a Library must have a user.
• It seems odd that "Index" is always 1
• It seems odd that a "Top Level" table does not seem to be purchased or personal.
• I would not flush the entity manager inside the those methods. Instead, I would let my calling code do it. The reason is that you may have other db operations to perform along with these. Postponing the flush operation allows you to do everything in 1 transaction.
• I renamed your method names to indicate that there could be side effects as a result of calling the method (get OR create).
• No need to persist parent. Persisting the sub (assuming associations are set up) should persist the parent.
• I think you'll always need at least 1 switch in there somewhere that is used to construct the new entity based on it's type.