current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 0 down vote favorite

I have read some articles regarding the factory pattern and decided to write some code as an exercise. This is my first attempt and am not 100% if I did it right. I've omitted some implementations for the sake of focusing on the concept at hand.

class NumbersFactory {
    public static function makeNumber( $type, $number ) {
        $numObject = null;
        $number = null;

        switch( $type ) {
            case 'float':
                $numObject = new Float( $number );
                break;
            case 'integer':
                $numObject = new Integer( $number );
                break;
            case 'short':
                $numObject = new Short( $number );
                break;
            case 'double':
                $numObject = new Double( $number );
                break;
            case 'long':
                $numObject = new Long( $number );
                break;
            default:
                $numObject = new Integer( $number );
                break;
        }

        return $numObject;
    }
}

/* Numbers interface */
abstract class Number {
    protected $number;

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

    abstract public function add();
    abstract public function subtract();
    abstract public function multiply();
    abstract public function divide();
}
/* Float Implementation */
class Float extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Integer Implementation */
class Integer extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Short Implementation */
class Short extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Double Implementation */
class Double extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}
/* Long Implementation */
class Long extends Number {
    public function add() {
        // implementation goes here
    }

    public function subtract() {
        // implementation goes here
    }

    public function multiply() {
        // implementation goes here
    }

    public function divide() {
        // implementation goes here
    }
}

$number = NumbersFactory::makeNumber( 'float', 12.5 );
share|improve this question

closed as off-topic by Brythan, vnp, RubberDuck, syb0rg, Abbas Jan 7 '15 at 10:16

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.
    
I have nothing to add to this code review, sorry, but I do have a question: How is this helpful in PHP. As a method of practice, sure, but (with this specific path) what are the real-world implications/benefits? – Mike Jan 6 '15 at 21:22
    
Decoupling, polymorphism, scalability and others. These are the OOP concepts that OOP was built for. – Robert Rocha Jan 6 '15 at 22:08
    
I mean, I get those concepts, to a degree. But, to me, I am having a hard time seeing the benefit of $one = NumbersFactory::makeNumber('int',1); $two = NumbersFactory::makeNumber('int',2); $three = $one->add($two); vs $three = 1 + 2; or $float_three = NumbersFactory::makeNumber('float', 3); vs $float_three = (float) 3; /* and */ $three = (int) $float_three;. For complex domains, I know the benefit to this style all to well, but your example seems very unnecessary, and confusing. To me, at least. – Mike Jan 6 '15 at 23:02
    
I get what you mean. It is unnecessary. It was simply an exercise of writing code that demonstrated the factory pattern concept. I likened this to a numbers library that one might have to compute among different numbers. The basis for this specific structure was gotten from the way Java implements Numbers: docs.oracle.com/javase/tutorial/java/data/numberclasses.html I could have used a different Factory, perhaps a car Factory. – Robert Rocha Jan 6 '15 at 23:23
    
msdn.microsoft.com/en-us/library/ee817667.aspx – Robert Rocha Jan 6 '15 at 23:24
up vote 4 down vote accepted

Coding Critique

public static function makeNumber( $type, $number ) {
    $numObject = null;
    $number = null;

Well, that almost certainly isn't right. You take a parameter which you then ignore, overwriting it locally. 

public static function makeNumber( $type, $number = null ) {
    $numObject = null;

That seems like it might be more useful. You accept a parameter if offered. Otherwise, you give it a default value. That may or may not be what you wanted to do. 

        case 'float':
            $numObject = new Float( $number );
            break;

You can make this simpler if you want:

        case 'float':
            return new Float( $number );

That way you can get rid of $numObject entirely. 

        default:
            $numObject = new Integer( $number );
            break;

You don't need a break; in the last clause. You can just let it fall through after a default: clause.

Factory Method

You have this tagged factory-method, but you aren't using a factory method. The factory method is when you have a method that can return different results depending on the exact class of the object. So you might have 

class CompositeNumberFactory {
    public static function makeNumber( $first, $second );
}

class FractionFactory extends CompositeNumberFactory {
    public static function makeNumber( $first, $second ) {
        return new Fraction($first, $second);
    }
}

class Fraction implements iNumber {
    private $numerator;
    private $denominator;

    function __construct($numerator, $denominator) {
        $this->numerator = $numerator;
        $this->denominator = $denominator;
    }

This gets more interesting if FractionFactory is called something else, e.g. RationalFactory. Then the caller may not even know that they have a Fraction. They just know that they have the normal iNumber methods.

Factory Pattern

There is a variant of the factory pattern that does work something like you specify, but it would normally have different types. E.g. 

        case 'decimal':
            return new Double( $number );
        case 'bit64':
            return Long( $number );
        case 'whole':
            return new Unsigned( $number );

This allows the user to not know what the various types are, just how the user would describe them. Also, this allows the underlying implementation to change without impacting the user. For example, if we moved from a 32-bit operating system to a 64-bit operating system, the second case might change from Long to Integer.

share|improve this answer
    
Thanks! The coding critique was excellent. What I had in mind was something similar to a numbers library for handling different number derivatives. I used this tree as the basis of this exercise from Java: docs.oracle.com/javase/tutorial/java/data/numberclasses.html – Robert Rocha Jan 6 '15 at 22:19

Not the answer you're looking for? Browse other questions tagged or ask your own question.