developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Ticket #523 (closed enhancement: fixed)

Opened 9 years ago

Last modified 8 years ago

Enhanced service management in jClasses

Reported by: doubleface Owned by: doubleface
Priority: normal Milestone: Jelix 1.1 beta 1
Component: jelix:utils Version: trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

Here is a proposition of enhancement for jClasses' service handling. It is inspired from Stubbles' inversion of control container but with less features and different but inspired from it. I tried to respect the "philosphy" of jelix or at least the vision I have of it. I think it is lightweight, fast, and not too much intrusive.

The goal of this feature is to choose the implementation behind an interface through jelix config file or through a special method call or through a special constant of the specified interface.

Here is a basic sample :

Jelix config file :

[Bindings]
modulename1*itfname = modulename2~targetclassname

Source code to use it :

$service = jClasses::getBindedService('modulename1~itfname');

Now suppose you are in a special environment and you want to use another implementation of the service without changing your source code. Then simply change the jelix config file to :

[Bindings]
;modulename1~itfname = modulename2~targetclassname
modulename1~itfname = modulename3~newimplementation

Now suppose you want to pass your unit test. If you use this service, a lot of things in your project are changed and you don't want it. Simply create a mock object with your unit test library (probably simpletest or PHPUnit) and use it for the service without even changing jelix config file:

    jClasses::inc('modulename2~targetclassname');
    $moke = createMoke('targetclassname');
    jClasses::bind('modulename1~itfname')->toInstance($moke);

These are some basic sample. There are more subtleties about it.

I have used this feature in my own project at work for 1 year and it is very useful especially for unit tests. I reworked it to be more general and to "fit" more with jelix.

Here is a patch with the feature added to jClasses, a new jBinding class, unit tests corresponding to it and a patch corresponding to ticket 522 (I needed it). I added also a parameter to jSelectorFactory::create to specify a default type of selector. I also updated the manifests. This patch is for the trunk (version 833).

At the moment, I'm not entirely satisfied with jelix config file handling. "~" characters are forbidden for keys in ini files. I had to replace it with a "*".

There are a lot of possible enhancements for it :

  • Make it work not only for services but for all classes, too.
  • Make real dependency injection by contructor, setter, but this needs the use of annotations (and then reflection which is slow), or other special constants (but I don't like it). See the stubbles implementation.
  • Use a specific config file and parser to allow "~" in keys. And this special config files could be compiled and cached.
  • This could be integrated in existing jClasses methods (create, getService) with one more parameter to it.

This is my first real attempt to enhance jelix. What do you think of it ? Any ideas ?

Attachments

bindings.diff (5.2 KB) - added by doubleface 9 years ago.
bindings.patch (14.6 KB) - added by doubleface 9 years ago.
bindings2.patch (23.6 KB) - added by doubleface 9 years ago.
bindings2.2.patch (23.6 KB) - added by doubleface 9 years ago.
bindings3.patch (23.4 KB) - added by doubleface 9 years ago.

Change History

Changed 9 years ago by doubleface

comment:1 Changed 9 years ago by laurentj

  • Owner laurentj deleted
  • review changed from review? to review-
  • Component changed from jelix:core to jelix:utils

Honestly, I don't know nothing about inversion of control. I have to study it :-)

I can't review your patch because you have missed all new files in your patch (do a svn add ;-)).

Please, provide a separate patch for line of codes which fix bug 522 and attach it to ticket #522. So I could review it quickly and integrate it in the repository, in both trunk and 1.0.x branch (you patch about binding will only be in trunk, if it's ok)

PS: do you want a write access on the repository ? If yes, subscribe on developer.berlios.de and tell me your login name.

Changed 9 years ago by doubleface

comment:2 Changed 9 years ago by doubleface

  • review changed from review- to review?

Oups, here is the good patch file, I hope (bindings.patch).

What I did is not exactly inversion of control, actually. This is just a kind of register of services that can be modified with config file or at runtime. If you look at the code or unit tests, you will see this is not too much complicated. I don't understand inversion of control fully either.

If the patch is integrated, I can explain all of this better in the wiki.

I would like a write access very much indeed ! My login name is doubleface on berlios too.

comment:3 Changed 9 years ago by doubleface

  • Owner set to doubleface
  • Status changed from new to assigned

comment:4 Changed 9 years ago by doubleface

  • Documentation needed set

comment:5 Changed 9 years ago by doubleface

Forgot to add a locale for error messages about bindings. THis is in bindings2.patch file

comment:6 Changed 9 years ago by doubleface

  • Type changed from bug to enhancement

comment:7 Changed 9 years ago by doubleface

Made it work with classes, too and not only services any more.

Changed 9 years ago by doubleface

comment:8 Changed 9 years ago by doubleface

Corrected some small mistakes in bindings2.2.patch

Changed 9 years ago by doubleface

comment:9 Changed 9 years ago by laurentj

  • review changed from review? to review-
  • Milestone set to Jelix 1.1 beta 1

lib/jelix/utils/jClasses.class.php

-class jClasses {
+class jClasses{

I prefer whand there is a whitespace before a {, it is more readable. Please cancel this modification and same others in your patch.

lib/jelix/utils/jBinding.class.php :

  • I'm not sure that defined constant in the class are really pertinent and usefull for external objects. Don't define them.
+    protected $fromselector = null;
+
+    protected $toselector = null;

Follow coding style (uppercase to separate words):

+    protected $fromSelector = null;
+
+    protected $toSelector = null;
+    public function getInstance() {
+        if ($this->instance === null){
+            if ($this->toselector === null) {
+                $this->toselector = $this->_getClassSelector();                

Carefull, many whitespace at the end of this line. Remove them.

+                // No '~' allowed as key of a ini file, we use '*' instead

Perhaps can we use '-' instead of '*' ? It could be better I think, and it is accepted by parse_ini_file.

You forgot to update the CREDITS file ;-)

Except this little errors, it's ok for me. Fix them and the next review will be ok :-)

comment:10 Changed 9 years ago by laurentj

Add also @since 1.1 in comments before each new methods in existing class or before each new classes.

Changed 9 years ago by doubleface

comment:11 Changed 9 years ago by doubleface

  • review changed from review- to review?

Here it is. You are right, '-' almost looks like '~'... I hope this one is the good one :-) ...

comment:12 Changed 9 years ago by laurentj

  • review changed from review? to review+

It's ok to commit this patch in the trunk :-)

comment:13 Changed 9 years ago by laurentj

  • Status changed from assigned to closed
  • Resolution set to fixed

Landed in the trunk.

comment:14 Changed 9 years ago by doubleface

Thanks for doing this for me laurent! I am not even near to get an internet access at home at the moment. But I will work on the documentation soon.

comment:15 Changed 8 years ago by laurentj

  • Documentation needed unset

Since the current implementation is too experimental (see ticket #778), we will create the documentation later.

Note: See TracTickets for help on using tickets.