Skip to main content
Code Review

Return to Answer

deleted 1 character in body
Source Link
  1. This is a creational pattern (it creates objects) - Your code satisfies this

  2. Objects get created without having to specify the exact class. The factory pattern deals with inheritance - You are specifying an exact class, and only have one class.

  3. A method is used to create the object

  4. Code using the "factory" doesn't call a constructor directiondirectly

  1. This is a creational pattern (it creates objects) - Your code satisfies this

  2. Objects get created without having to specify the exact class. The factory pattern deals with inheritance - You are specifying an exact class, and only have one class.

  3. A method is used to create the object

  4. Code using the "factory" doesn't call a constructor direction

  1. This is a creational pattern (it creates objects) - Your code satisfies this

  2. Objects get created without having to specify the exact class. The factory pattern deals with inheritance - You are specifying an exact class, and only have one class.

  3. A method is used to create the object

  4. Code using the "factory" doesn't call a constructor directly

added 244 characters in body
Source Link

Your code satisfies points 3 and 4, with one fatal problem: The createOrganization method also INSERTs a record into the database. If a factory method is written correctly, the only thing it does is create an object. A factory method or class should never do anything else.

Your code satisfies points 3 and 4.

Your code satisfies points 3 and 4, with one fatal problem: The createOrganization method also INSERTs a record into the database. If a factory method is written correctly, the only thing it does is create an object. A factory method or class should never do anything else.

Source Link

No, this is not 100% true to the Factory Pattern - almost, but not 100%.

From Wikipedia:

In class-based programming, the factory method pattern is a creational pattern (1) that uses factory methods to deal with the problem of creating objects without having to specify the exact class (2) of the object that will be created. This is done by creating objects by calling a factory method (3)—either specified in an interface and implemented by child classes, or implemented in a base class and optionally overridden by derived classes—rather than by calling a constructor (4).

(emphasis, mine)

The key points here are:

  1. This is a creational pattern (it creates objects) - Your code satisfies this

  2. Objects get created without having to specify the exact class. The factory pattern deals with inheritance - You are specifying an exact class, and only have one class.

  3. A method is used to create the object

  4. Code using the "factory" doesn't call a constructor direction

Your code satisfies points 3 and 4.

The major reason why you haven't actually created a "factory" is because you do not utilize inheritance, where you create a concrete instance and return it as an abstract parent class or interface.

Instead, what you have created is a Data Access object.

Now, a data access object can utilize a factory. Let's pretend your Organisation class is abstract, and it has two concrete child classes:

  • Business
  • NonProfitOrganisation

Sample code:

class OrganisationDataAccess
{
 private function createOrganisation($data) : Organisation {
 if ($data['type'] == 'business') {
 return new Business($data['ID'], $data['Name'], $data['Created']);
 } else if ($data['type'] == 'non-profit') {
 return new NonProfitOrganisation($data['ID'], $data['Name'], $data['Created']);
 }
 throw new Exception('Invalid organisation type: ' + $data['type']);
 }
 public function getOrganisation($id) {
 $data = // get from database
 $org = $this->createOrganisation($data);
 // Populate $org with more data
 return $org;
 }
}

Now the createOrganisation class is a true factory method. It instantiates concrete child classes and returns them as the abstract parent.

Now, simply because you aren't 100% true to the definition doesn't mean your createOrganisation method isn't useful - it eliminates duplication of code when creating new objects. But I would suggest a name change for your class: OrganisationDataAccess or OrganisationRepository is a better fit.

If, in the future, you really do need sub classes you have the option of adding an OrganisationFactory class if need be.

lang-php

AltStyle によって変換されたページ (->オリジナル) /