What about using traits to split code from a big class into several files?

Question

Coming from Ruby, I was all excited to discover PHP traits. They resemble ruby's module system and I like it.

Now I've read lots of posts saying PHP traits are evil, because they break OO design, you can't test them as easily as you would a class, etc.

I'm wondering if using traits to split a very big class into more readable parts would be a good idea.

The class is already big, and will likely end up thousands of lines long without re-factorization.

Even though methods can be grouped by concern, they constantly need to call one another, which makes me wonder whether it is optimal to split the class into different classes.

To illustrate, here is a simplified example I've come up with.

The big class:

/**
* Class to refactorize
*/

class Tourist
{
    public function gotoRestaurant()
    {

    }

    public function gotoInternetCafe()
    {

    }

    public function checkEmails()
    {

    }

    public function meetGirlfriend()
    {
        if (Girlfriend::location() === 'restaurant')
        {
            $this->gotoRestaurant();
        }
        else
        {
            $this->checkEmails();
        }
    }

    public function checkEmailsAndMeetGirlfriend()
    {
        $this->checkEmails();
        $this->meetGirlfriend();
    }
}

Here is how I'd do it if re-factorizing with classes:

/**
* Solution 1: using classes
*/

class ChoreDoer
{
    public function __construct(Tourist $t)
    {
        $this->tourist = $t;
    }

    public function checkEmails()
    {
        $mover = new Mover($this->tourist);
        $mover->gotoInternetCafe();
    }

    public function meetGirlfriend()
    {
        if (Girlfriend::location() === 'restaurant')
        {
            $mover = new Mover($this->tourist);
            $mover->gotoRestaurant();
        }
        else
        {
            $this->checkEmails();
        }
    }
}

class Mover
{
    public function __construct(Tourist $t)
    {
        $this->tourist = $t;
    }

    public function gotoRestaurant()
    {

    }

    public function gotoInternetCafe()
    {

    }
}

class Tourist
{
    public function checkEmailsAndMeetGirlfriend()
    {
        $cd = new ChoreDoer($this)
        $cd->checkEmails();
        $cd->meetGirlfriend();

    }
}

I've got a feeling constantly calling new Mover(), new ChoreDoer(), and the new classes that will appear is going to become painful to read and write.

So what about using traits here to group functionality?

/**
* Solution 2: using traits
*/

trait TouristMovements
{
    public function gotoRestaurant()
    {

    }

    public function gotoInternetCafe()
    {

    }
}

trait TouristChores
{
    public function meetGirlfriend()
    {
        if (Girlfriend::location() === 'restaurant')
        {
            $this->gotoRestaurant();
        }
        else
        {
            $this->checkEmails();
        }
    }

    public function checkEmails()
    {
        $this->gotoInternetCafe();
    }
}

class Tourist
{
    use TouristMovements, TouristChores;
    public function checkEmailsAndMeetGirlfriend()
    {
        $this->checkEmails();
        $this->meetGirlfriend();
    }
}

Here each trait can focus on its purpose, the code is easy to read and methods can inter-operate freely. But since I never intend to use the TouristsMovements trait in any class other than Tourist, I have a feeling my use of them is not really what traits were designed for.

Would you go with solution 1? solution 2? something else?

Answer 1

As an alternative, you could do something like this, using your 'Solution 1':

class ChoreDoer
{
    private $mover;

    public function __construct(Tourist $t)
    {
        $this->tourist = $t;
    }

    public function checkEmails()
    {
        $this->getMover()->gotoInternetCafe();
    }

    public function meetGirlfriend()
    {
        if (Girlfriend::location() === 'restaurant')
        {
            $this->getMover()->gotoRestaurant();
        }else
        {
            $this->checkEmails();

        }
    }
    /**
     * lazy-load mover object
     */
    private function getMover()
    {
        if($this->mover == null)
        {
            $this->mover = new Mover($this->tourist);
        }
        return $this->mover;
    }
}

If you don't fancy using the getter, you could instead name your property something like

private $_mover;

And use a magic __get to call your getter for you:

function __get($property)
{
    $method = 'get'.ucfirst($property);
    if(method_exists($method))
    {
        return $this->$method();
    }
}

Although this will not perform quite as well, it makes it easy to just call

$this->mover->gotoInternetCafe();

And is nice and clean.

Answer 2

I'm curious as to what the OP ended up doing.

Currently in the same situation. I have a huge class with lots of methods doing different things.

At first I thought of splitting them into their own standalone classes but that required a lot of replacing $this->xxx() calls with $actualobj->xxx(). Good side of this method is I don't need to load/include all classes at once as I can simply autoload what's needed.

Second solution was to use traits in which case the whole thing will still be loaded into memory but at least the code is more organized, readable and easier to debug being separated into their own files/classes.

I decided to do it in 2 phases where I do traits first so as just to separate and cleanup the code then change each trait to standalone classes and replace necessary $this->xxx() calls with $actualobj->xxx() calls down the road.