-3

I made a code that seems to mix Singleton design pattern, and Fatory method. But my factory method is in an abstract class inherited by my Singleton ... what the hell have I created ? Does it have a name ?

Here is the situation (in PHP I hope to be understandable by everyone) :

I had a circular inclusion problem :

class ThingsService {
    private $stuff_service ;
    public function __construct () {
        $this->stuff_service = new StuffService () ;
    }
    public function f () { $this->stuff_service->f() ; }
    public function g () { echo "Hello world !\n" ; }
}
class StuffService {
    private $things_service ;
    public function __construct () {
        $this->things_service = new ThingsService () ;
    }
    public function f () { $this->things_service->g() ; }
}

$toto = new ThingsService () ;
$toto->f() ;

Creating a ThingsService lead to creating a StuffService which lead to creating a ThingsService ... and so on.

Due to time development constraints, it wasn't possible to change the codes of f and g methods. Due to how my code works, it was possible to implement StuffService and ThingsService as Singletons. Due to the fact that they were both "Services", it was possible to factorize some code with some inheritance. Hence, I did this :

abstract class Service
{
    private static $_instances = [];
    public static function get_instance ()
    {
        $required_class = get_called_class () ;  // "StuffService" or "ThingsService"
        if (!isset (self::$_instances[$required_class]))
        {
            new static () ;
        }
        return self::$_instances[$required_class] ;
    }
    protected function __construct ()
    {
      $required_class = get_called_class () ;
      self::$_instances[$required_class] = $this ;
    }
}
class ThingsService extends Service
{
    private $stuff_service ;
    protected function __construct ()
    {
        parent::__construct () ;
        $this->stuff_service = StuffService::get_instance () ;
    }
    public function f () { $this->stuff_service->f() ; }
    public function g () { echo "Hello world !\n" ; }
}
class StuffService extends Service
{
    private $things_service ;
    protected function __construct ()
    {
        parent::__construct () ;
        $this->things_service = ThingsService ::get_instance () ;
    }
    public function f () { $this->things_service->g() ; }
}

$toto = ThingsService::get_instance () ;
$toto->f() ;

Edit due to subsequent answers

In the actual code, there are more than 2 XxxxxService class: there are actually 15 of them, each containing between 500 and 2000 lines of code. This interdependency is the result of a not well thought out architecture that wasn't good enough to support the recent augmentation of the code size. It is not possible to merge them in one file due to this huge amount of lines.

As I told earlier, this new architecture is also constrained by the time we can invest in this refactoring (refactoring 15 constructor is possible, but changing 235 method calls everywhere in the code isn't likable).

So my questions are:

  • Is this a known design pattern ?
  • If yes, does it have a name ?
  • Does this code design seems ok to you ? Or do you think it's a dirty or problematic code (and why) ?
  • 1
    It's called the Circular Dependancy design pattern and should be refactored out. – Pieter B Nov 27 '19 at 13:52
  • @PieterB Yes, that's what the answer and comments bellow are already saying. Thanks for your comment though. – Motiss Nov 27 '19 at 14:25
  • Is this real code, or just example code that you wrote to illustrate the problems? – Robert Harvey Nov 27 '19 at 15:12
  • @RobertHarvey Just an example to illustrate the design. – Motiss Nov 27 '19 at 15:20
  • Foobar examples seldom make good illustrations. Your example code is totally stripped of any business domain information, and it's difficult to make any recommendations when we don't know what it is supposed to accomplish. – Robert Harvey Nov 27 '19 at 15:31
2

If "ThingsService" & "StuffService" have such coupling (i.e. they both call each other's methods), why you found the need to separate them to two distinct classes in the first place ?

I advise you to reconsider your design decisions, either create a single class or move out the shared functionality between the classes to a separate class and inherit from it.

Regarding your questions:
It's not a design pattern per se.
A factory can be a singleton.
This is a design smell, as the two classes & the phrasing of the question indicate a strong coupling between them.

Regarding details in comment:
Having a 15000 lines of class, and redesign it to multiple classes with a very strong coupling - is not a solution to dealing with its complexity, and it can only create maintenance headaches down the road.
Again, you should reconsider your design of this class - separating shared functionality to an encapsulated class, composition etc.
Regarding the memory question, you can employ lazy schemes in your class and allocate memory only when you need it. (and you can deallocate it when you don't need it anymore). (Instead of eagerly allocating it in the constructor)

|improve this answer|||||
  • I separated the Services for 2 reasons : code size and memory. I actually have 15 XxxxService class, each one of them containing from 500 to 2000 lines of code, and we'd like to avoid a 15000 line file if possible. Then, each Service will instantiate different resources in its constructor, which cost memory, separating Services was also a lazy way to prevent useless memory allocation. I understand you point tho, and we should definitively consider it for future code refactoring. – Motiss Nov 27 '19 at 11:32
  • I don't understand what "This is a design smell" means in English. (And online translators aren't helpful at all.) Could you rephrase your sentence please ? Thanks. – Motiss Nov 27 '19 at 11:34
  • 2
    @Motiss: A "smell" (in this context) means "it suggests something is wrong but I can't put my finger on the exact mistake". It's a fairly common saying in the field of software engineering, useful whenever you see something is wrong but the specific solution requires more context. – Flater Nov 27 '19 at 11:42
  • 1
    @Motiss: Breaking up a monolith class is good, but the way in which you're breaking it up isn't perfect. Right now, all you're doing is putting the code in different files but still having it all functionally depend on each other by making all of these calls (which is what lead nadir to suggesting that these classes are too dependent on each other to be separated). You should instead be encapsulating each individual class so that it doesn't need to rely on a complex dependency structure. – Flater Nov 27 '19 at 11:44
  • 1
    Your first comment should've been in your question, as it requires a detailed answer for your case. I will edit my question with an answer to it - I suggest that you edit your question and add this details. – nadir Nov 27 '19 at 11:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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