Where to implement a factory method?

Question

I've been trying to grasp OOP concepts and while I do get the general ideas behind most of them, I often find myself in need of some advice regarding their practical implementation. One of such cases is the factory method.

I'm writing a PHP app that's going to handle requests incoming from both web and command-line interface, so I came up with the following simple class inheritance structure to cover both types of requests:

abstract class Request {
}

class HttpRequest extends Request {
}

class CliRequest extends Request {
}

Now I need a factory method that would return a concrete Request instance, depending on the value returned by php_sapi_name():

public function create() {
    if(php_sapi_name() === 'cli')
        return new CliRequest();
    else
        return new HttpRequest();
}

My question is: where do I put it? I can think of at least three possibilities:

1) Static method in a separate class:

class RequestFactory {
    public static function create() {
        // ...
    }
}

2) Regular method in a separate class (would require instantiating the class first):

class RequestFactory {
    public function create() {
        // ...
    }
}

3) Static method in the abstract parent class:

abstract class Request {
    public static function create() {
        // ...
    }
}

What are the pros and cons of each solution and which would be considered "proper" and why?

Answer 1

To create truely loosely coupled code you could use Ray.Di or Injektor and do something similar to the following:

<?php

use Ray\Di\Di\Inject;
use Ray\Di\Di\Scope;

/**
 * @Scope("Singleton")
 */
abstract class Request {
}

class HttpRequest extends Request {
}

class CliRequest extends Request {
}

class ARequestConsumer {
    /* @var Request */
    private $request;

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

    public function foo()
    {
       //...
    }
}

class Global extends Ray\Di\AbstractModule {

    public function configure()
    {
        $this->bind( 'Request' )
            ->toProvider( 'RequestProvider' );
    }
} 

class RequestProvider implements \Ray\Di\ProviderInterface {
    /**
     * @return Request
     */
    public function get()
    {
       //.. factory method logic goes here that produces a concrete instance of Request

    }
}



$injector = Injector::create([new Global]);
$consumer = $injector->getInstance('ARequestConsumer');
$consumer->foo();

Answer 2

Design patterns are not restricted to OOP and a lot of implementations of OOP design patterns are written with some memory management in thought.

I come from the Java world and in Java you would have to use a strict OOP design pattern. Simply because everything in Java is an object. Sometimes you must create an object and a method even if it is actually not needed for the pattern itself. The factory method design pattern is such an example. It is very good to design by interface for the implementations of the factory, but you don't need a class and method to implement the factory. The reason that an implementation of a design pattern is sometimes confusing is that the programming language sometimes requires an implementation that is not strictly needed in the design pattern itself. The creation of a class with a method in the factory method is such an example.

My solution is not purely OOP, but PHP is that not too and in the long run is it not about OOP in my opinion, but about the best implementation of the design pattern factory method.

I think that the elegancy of PHP is that it combines best of both worlds. It delivers a solid OOP design possibility, yet it has not thrown away the good elements of procedural programming.

You can simply create code like this:

function createRequest($pRequesttype){
  switch($pRequesttype){
    case "cli":
      $tmp = new CliRequest();
      break;
    case "http":
      $tmp = new HttpRequest();
      break;
    default:
      $tmp = new DefaultRequest();
  }
  return $tmp;
 }

Always return a default implementation to handle the request. A switch statement is the best choice to extend the number of choices in a software engineer friendly way.

Now have you the call php_sapi_name in your create function. I advice you to get it out the implementation of the function. It is best practice to let a function do one job only and getting the request and handling the request are two functions. Make a createRequest function that has a parameter like I showed you.

To answer your question: 1, 2 or 3? Uh, 4 actually. :-) If 1, 2 or 3?

Definitely 1, because I don't want to load too much classes for a simple method, but to simplify this situation I have proposed solution 4.

I would not use method 2, because it is not efficient to create a class for one moment in time. I would consider it best practice, but not the best practical implementation. If using this solution, then please also support the class with an interface for factories in general. Best OOP design, but not best practical implementation.

I would certainly not use method 3, because abstract classes are there to abstract data objects and interfaces to abstract behaviour. A factory method is an abstraction of an interface, never of an abstract class.

Answer 3

So, I have an idea, to do what I think you want, using method overloading.

class Request {

    private $request;
    private $valid = true;
    private $type;
    private $vars;
    private $methods;

    public function __construct() {
        $this->type = php_sapi_name() === 'cli' ? 'cli' : 'http';
        if($this->is_cli()) $this->request = new CliRequest();
        else if($this->is_http())  $this->request = new HttpRequest();
        else {
            $this->valid = false;
            return;
        }
        $this->vars = get_class_vars($this->request);
        $this->methods = get_class_methods($this->request);
    }

    public function __get( $var ){
        if(!$this->valid) return false;
        if(!in_array($var, $this->vars)) return false;
        return $this->request->$var;
    }

    public function __set( $var , $val ){
        if(!$this->valid) return false;
        if(!in_array($var, $this->vars)) return false;
        return $this->request->$var = $val;
    }

    public function __call( $meth, $args ){
        if(!$this->valid) return false;
        if(!in_array($meth, $this->methods)) return false;
        return call_user_func_array($this->request->$var, $args);
    }

    public function is_cli( ){
        return $this->type == 'cli';
    }

    public function is_http( ){
        return $this->type == 'http';
    }


}

// Then, when calling the function...
$request = new Request;
$request->variable; // will get the variable from the Cli or Http request class
$request->method("a","b","c"); // Will run the method from the Cli or Http request class

Answer 4

All these possibilities will work as expected. I don't really get any "cons" as it fulfils, what is IMHO, your encapsulation objective. Now, let's look at Factory Method pattern essence:

Define an interface for creating an object, but let the classes that implement the interface decide which class to instantiate. The Factory method lets a class defer instantiation to subclasses.

I'm not sure if what you're willing to do perfectly matches this definition.

Instead, it looks like you want to implement something called "Simple Factory" in which instantiation process is encapsulated into a class.

But having this kind of method directly into the abstract class that defines the interface of your "Request" objects doesn't look like a bad idea.

As Nicolas said, it's a rather common pattern in Java, C#, and Cocoa lands.

For these reasons, my choice would go to 3rd option

Answer 5

In this case I rather use the Base Abstract class as the creator of the Instance using the static method (3rd Option)

I will use an external class, like in the first option when I need to create some dependencies that Can break the encapsulation like having different dependencies for different implementations. and will turn the class less maintainable.

Answer 6

Using a static method in the parent class doesn't seem an awful solution to me at all. Take a look at the Calendar class in Java: There's a getInstance method (many actually) which return a Calendar instance depending on your Locale, and some others criteria.