Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Splitting a large PHP class

I have a large class (1500 lines, but will soon be several times that) that I'd like to split so that it fits better with SRP (and so that each file is smaller and more manageable.)

The class contains 50-100 properties, and has several different types of actions that are performed against it - one of which would be an update which in turn does several steps such as update the database, and send emails.

So I think I want 4 classes.

How should I structure the classes?


Here's a simplified version of what I have now:

class Foo {
    public function __construct ($params) {}

    public function update () {
        $this->updateDatabase();
        $this->sendEmails();
    }

    private function updateDatabase () {}
    private function sendEmails () {}
}

$foo = new Foo($params);
$foo->update();

updateDatabase() and sendEmails () each call many other methods - hundreds of lines of code each, and they have several sibling methods doing other tasks.

A basic rewrite to use static methods

class Foo {
    public function __construct ($params) {}
}

class FooUpdate {
    public static function update ($fooParam) {
        FooUpdateDatabase::main($fooParam);
        FooSendEmails::main($fooParam);
    }
}

class FooUpdateDatabase {
    public static function main ($fooParam) {}
}

class FooSendEmails {
    public static function main ($fooParam) {}
}

$foo = new Foo($params);
FooUpdate::update($foo);

A basic rewrite to use instantiated objects

class Foo {
    public function __construct () {}
}

class FooUpdate {
    private $foo;
    public function __construct ($fooParam) {
        $this->foo = $fooParam;
    }
    public function main () {
        $fooTemp = FooUpdateDatabase($this->fooParam);
        $fooTemp->main();
        $fooTemp = FooSendEmails($this->fooParam);
        $fooTemp->main();
    }
}

class FooUpdateDatabase {
    private $foo;
    public function __construct ($fooParam) {
        $this->foo = $fooParam;
    }
    public function main () {}
}

class FooSendEmails {
    private $foo;
    public function __construct ($fooParam) {
        $this->foo = $fooParam;
    }
    public function main () {}
}

$foo = new Foo($bar, ...);
$fooTemp = new FooUpdate($foo);
$fooTemp->update();

Or should I use inheritance or traits somehow?

like image 897
Redzarf Avatar asked May 18 '16 16:05

Redzarf


2 Answers

As @SparK says,

  • Your object (Foo)
  • A class that handles database communication (FooRepository)
  • A class that sends mails (Mailer)
  • A class that wraps it all (FooManager)

    $foo = new Foo($params);
    $fooManager = new FooManager(FooRepository, Mailer);
    $fooManager->update($foo);
    $fooManager->notify($foo); //this could be inside the update or an event.
    

This way you can also decompose your classes (ie: separate a class that handles database connections and inject it in the FooRepository, etc). But I don't think having classes that represent an action is the way to go?

Classes are objects that can do actions (among other things), not actions (this is just a comment because of the names you used in your example :p).

like image 76
Blitu Avatar answered Nov 09 '22 20:11

Blitu


I guess sending e-mails is one thing, representing your data is another and read-write operations to database would be a third.

So class Foo, class FooPersistence, class FooMailer.
And whoever calls the FooPersistence::update($foo) should also call FooMailer::sendUpdateNotification($foo).

As a side note:
If you had something like Events set up I would trigger the "update event" inside the persistence class and add a listener to it which sends e-mails.

like image 34
SparK Avatar answered Nov 09 '22 20:11

SparK