CODEWITHSUNDEEP
phplaravellaravel-5
BigJobbies
BigJobbies

Reputation: 4055

PHP - Using Interfaces, Strategy Pattern and Optional Method Parameters

I was wondering if i could get a bit of help.

I have an interface like so

interface BackupContract {
    public function testConn($request, $port);
}

Then the 2 example implementations of this interface is as follows

class FTPBackup implements BackupContract {
    public function testConn($request, $port = 21) {
        // code here
    }
}

class SFTPBackup implements BackupContract {
    public function testConn($request, $port = 22) {
        // code here
    }
}

As i need things like 'service' and port designated at runtime, im using 'strategy pattern' to achieve this, like so.

class BackupStrategy {
    private $strategy = NULL;

    public function __construct($service) {

        switch ($service) {
            case "ftp":
                $this->strategy = new FTPBackup();
                break;
            case "sftp":
                $this->strategy = new SFTPBackup();
                break;
        }

    }

    public function testConn($request, $port)
    {
        return $this->strategy->testConn($request, $port);
    }
}

and finally, in my controller im using the following code to put it all together.

$service = new BackupStrategy($request->input('service'));
$service->testConn($request, $request->input('port'));

The problem is, that if a user doesnt enter a port, it is meant to auto assign a port variable, i.e 21 or 22 like in the 2 implementations.

It doesn't seem to be working, but its not throwing any errors

Upvotes: 2

Views: 2515

Answers (4)

Lautaroml86
Lautaroml86

Reputation: 51

But you shouldnt implement the switch in the BackupStrategy's constructor.

With your aproach, you are violating the "open/close" principle from the S.O.L.I.D 's principles.

Any time you need to add another "backup strategy" you need to modify the BackupStrategy class, adding the logic to the constructor. Thats not good.

You could make sublcasses, each implementig the implementation (LOL) of each backup strategy classes and only inherating the necessary from BackupStrategy class.

interface BackupContract {
    public function testConn($request, $port);
}

class FTPBackup implements BackupContract {
    public function testConn($request, $port = 21) {
        // code here
    }
}

class SFTPBackup implements BackupContract {
    public function testConn($request, $port = 22) {
        // code here
    }
}

class BackupStrategy
{
    private $strategy;

    public function testConn($request, $port)
    {
        return $this->strategy->testConn($request, $port);
    }
}

class ConcreteFTPBackup extends BackupStrategy
{
    function __construct()
    {
        $this->strategy = new FTPBackup();
    }
}

class ConcreteSFTPBackup extends BackupStrategy
{
    function __construct()
        {
        $this->strategy = new SFTPBackup();
    }
}

$service = new ConcreteFTPBackup();

$serice->testConn($request, $request->input('port'));

Or even this:

interface BackupContract {
    public function testConn($request, $port);
}

class FTPBackup implements BackupContract {
    public function testConn($request, $port = 21) {
        // code here
    }
}

class SFTPBackup implements BackupContract {
    public function testConn($request, $port = 22) {
        // code here
    }
}

class BackupStrategy
{
    private $strategy;

    function __construct(BackupContract $bc)
    {
        $this->strategy = $bc();
    }

    public function testConn($request, $port)
    {
        return $this->strategy->testConn($request, $port);
    }
}


$service = new BackupStrategy(new FTPBackup());

$serice->testConn($request, $request->input('port'));

Then you could implement the switch during runtime.

Also you could make a setStrategy method in BackupStrategy class that set or change the backup strategy during runtime:

public function setStrategy(BackupContract $bc)
{
    $this->strategy = $bc();

}

So now, you can create a service with one backup strategy during runtime, and even change the strategy during runtime also!! Chekit out:

$service = new BackupStrategy(new FTPBackup());
$service->testConn($request, $request->input('port'));


$service->setStrategy(new SFTPBackup());
$service->testConn($request, $request->input('port'));

The BackupStrategy class is where all your encapsulated algorithms will converge, but don forget the "encapsulation" in all this!

The most important in the Strategy Pattern is the encapsulation of a family of algorithms that can be used during runtime!

Hope it helps you!

Upvotes: 5

schellingerht
schellingerht

Reputation: 5806

In addition to Simon and Laurent.

You're using interfaces, so the implementation should match.

A possible solution:

interface BackupContract {
    public function testConn($request, $port = 0);
}

But personally, I don't prefer this approach. Optional things in interfaces means that you should validate in possible each implementation instead of trusting the implementation.

Secondly, I recommend using type declarations (PHP7), for example:

public function foo(int $bar) : bool
{
    return true;  
}

This method expects an integer as argument (must) and a boolean value as return (must). Using $bar, you are sure that this var type is an integer.

See for more information: http://php.net/manual/en/migration70.new-features.php

Upvotes: 1

Laurent Meganck
Laurent Meganck

Reputation: 191

You are always setting a port in BackupStrategy->testConn($request, $port) method. So if somebody doesn't give a port, it will pass an empty string.

I would implement a fallback function.

And what @simon said, you are using the wrong method name in your controller.

Upvotes: 0

simon
simon

Reputation: 2946

You are calling a testConnection(); method which doesn't exist. What you actually want to call is testConn();

Check your error reporting settings because this should definitely throw an error.

Upvotes: 0

Related Questions