User tests: Successful: Unsuccessful:
OK, version 2 of my new approach for testability.
Gone is the glue file, replaced by a mechanism that is almost dependency injection, letting the testing fixtures supply the test doubles for the calls. (There's a baseline feature improvement I've created for my own use of this module, which I'll file a PR to -- or add to this one -- once I know if this approach is accepted.)
This means the actual code is shorter than with the glue file, but a handful of lines longer than current production, added to accommodate the dependency injection properly.
I'd appreciate it if someone besides me would test this out and make sure it's serviceable. I've written a fairly complete set of unit tests for it, but there's always the possibility I've either overlooked something or made a wrong assumption; the more eyes, the less likely that will happen.
The main advantage of this approach is it gives us a "bridge" out of static-land. It's a way to slide tests under existing code to help us take some of the speed bumps out of the transition. It increases code complexity to do it, so it's not a solution, merely a temporary safety net.
Side note: In my standalone version of this code, I did not inherit from JModuleHelper, but in this version I did. (My standalone version exists to prove the point I could create a module with 100% test coverage without loading one byte of Joomla code.) Doing that required a single change to the default value of one of the optional constructor arguments. The object of decoupling is to make changes that easy, or even easier.
It's hardly an issue, Nick. It's more a proposal for the future direction of writing project modules. Since the code is here, it seemed fit the discussion should be as well. If you want to point to it from somewhere else, feel free; I'll be doing the same. I don't want to suddenly start rewriting modules on everybody without having some sort of consensus it's the best direction for the near future, and it's easier to have that discussion remain focused if it's done in the presence of the code, so we can all see what everyone is talking about.
Ah, OK. So would you like me to label this as a Request for Comment?
Fine. If we get consensus, then we can pull it in. But since it's going to touch every module, I'd rather talk about it and beat it up some more before we get to "shall we pull it in?"
For one thing, This absolutely doesn't work in PHP 5.2, so it can't go in 2.5.x (but 3.0.x still says "at least 5.3" right?)
Hmmm. For starters, there is no glue class in this version.
But there's currently a serious issue with that approach: the document renderer has no idea what information the module's constructor needs in order to create itself. Since we're going to have to assemble that info on a per-module basis, the existing startup code may as well remain. Once we've laid out a large number of modules some sort of unified startup code may emerge. Then again, that might not emerge until we've made changes deeper in core.
That's not to say we never will do it that way. Once the whole system moves forward from static-land we might want to simply instantiate the module object(s) with a way to discover the info they need for themselves. But I think perhaps that's a decision best left until we know better what shape a truly object-oriented Joomla might look like. In the mean time, this can get us started down the road to change. I'm looking forward to finding out what a CMS built on objects working together will look like; I don't want to close off too many options when we're this far away from that goal. Let's get a few more courses of bricks laid first.
That would have been a good guess on my part -- didn't see the other version!
Paladin@73bcb91
That's the first link in your patch?
Just looked at your patch.
My comments weren't clear. I was definitely not expecting the rendering to do anything more than look for certain filename conventions and instantiate the helper, passing it in to the constructor if it exists.
The renderer doesn't need to know anything - the helper does - and the helper becomes the driver (the controller.)
But, not a big deal, let me watch you continue and if I get a moment - I'll try it out and show you.
Love what you have done.
The JoomlaGlue file was part of the first pass. I dropped it completely in favor of the static injection system when I saw that j3 required php5.3, because that was less complex and more scalable, so that file isn't in this. Its dropped in Paladin@86b4707. I could have probably made the commit package neater with a good rebase; sorry about that.
Given the current state, this helper is the module, hence my confusion about what you're saying.
I think the cleanest long-term goal is for the renderer to just tell the module object to do its thing ("Tell, don't ask") but we're too far away from that to work on it now. A lot of how that will work will depend on how the framework evolves, so I'm willing to wait and see. Given how outlandishly complex some of the module helper files are (I saw one with an NPath complexity of several billion -- shouldn't be more than a few hundred) it's not like we'll run out of things to do in the meantime.
(NPath complexity, for those playing along at home who might not catch it, is a measure of how easy it is for you/me to keep what's happening in this method in our heads. It's based loosely on the number of possible execution paths through the code, with a few penalties thrown in for tricky path variants. Example: an if/else has two paths; put two in the same file you get four, put another one in and you double again to eight, etc. I've seen nested if/else statements inside nested switch statements; I'm fairly certain no human keeps all the possible paths in their head while working on it, therefore any work done on it is more error-prone than if the code were kept simpler.)
If I were attacking the problem in the manner you're talking about right now (if I understand it properly) I'd reverse that slightly.
1) Renderer looks for module.php file. if found, it tells it to build itself and create its output (name for the output method can be almost anything, even if I'm biased against "execute"). Two statements.
2) Module constructor tells helper to build itself. Helper's job is to gather the information from various sources and have it ready to hand when module tells it to do something.
If we were to look at this specific example, module.php probably has a simple constructor and the createOutput method. Pretty much everything else is in helper. Unfortunately, this approach can couple the module and helper too closely for comfort.
My own preference would be to shoot the module helper class. No one knows ahead of time what other objects a module will require, if any, so it's foolish to decide ahead of time on a helper class -- let each module build the specific classes it needs. In many ways, I think helper classes are a design smell; there are better ways of doing it and if you have to resort to a generic helper, you're already losing. My opinion, YMMV.
This approach would require either the module discover its required information for itself, or it be passed in the means to find it. Right now the former requires way too many statics; eventually it may only require a single object be passed in, and the same object to all extensions, making it easier for the renderer to carry the load of creating them itself.
| Build | ⇒ | . |
| Status | New | ⇒ | Pending |
| Category | ⇒ | Modules |
This PR could be called ancient. I have to admit that I did not read the whole thing in complete detail, but I think I like the approach. In any case, what I don't like is the unit testing with the mock registry class. You can simply use a Registry object instead. That would be a lot better than having yet another stub in there.
Besides that: Is this still current? If so, could you update the codebase to the latest commits?
While I might agree with the approach, I have to say that I heard about this only now and never before. It would be good to bring this to the mailinglists, simply so that more people are made aware of this discussion here.
It was mentioned back on the framework list when I did it. I also presented it at Joomla Day New England and Joomla Day Midwest in Milwaukee (the Milwaukee presentation was flawed, as it focused on using this approach for testing JObservable/JObserver, as doing so would point out they were broken, which created a second battle to be fought at the same time, the reason I switched to using it with modules) but I'd have been surprised if word of any of those presentations made it to Germany. My memory fails me at the moment as to whether I wrote something in the Joomla Community Magazine as well for it. I intended to, but it’s been way too long since then for me to remember. Nick (above in the thread) volunteered to make it an RFC as well. I figured, mistakenly it appears, those were sufficient attempts at publicity.
Most efforts to modernize the CMS code into something easier to maintain and develop extensions for were discouraged, because doing so would mean extensions might need to change, so I dropped this idea, as well as several others I had in the hopper; it wasn't worth the battle, and still isn't. The interesting bit is the project doesn't need to accept it for independent extension developers to build their code this way. It worked with the contemporary CMS codebase.
Anyway, because I dropped the idea (and forgot to kill the PR, it appears) the code is not expected to work with anything past 2.5.x, other than accidentally.
Registry v. mock: It's a matter of preference whether the Registry object approach is better than the mock class. I preferred the mock approach simply because I didn't want any part of Joomla injecting itself into the test besides the specific bit I was testing unless there was positively no other way to run the test; I wanted to be able to code and test without having to load all (or any, preferably) of Joomla every time (an approach that would be possible, even normal, in properly decoupled software) and it was easier to do that in the world of modules than it was in any particular part of the existing CMS. But I recognize that sort of "purist" preference for isolation isn't shared by everyone, and I wouldn't insist on it. In fact, were I to do it over again, I'd probably do a lot of it differently anyway.
The initial aim here was as much to give module developers a way to do automated testing and development on their modules without having to keep fiddling with browsers and Joomla installs (surely a more tedious workflow for developing software couldn't be invented if we tried?) as it was to make it into the CMS. An extension developer could use this approach to building their software using modern TDD techniques without the project ever accepting this into core. I was hoping it would engender some discussion, and I intentionally put it up in a rough form because I wasn't sure what would be either the best, or most palatable, way for it to proceed. Amy had some good input to it, but no one else -- here, on Twitter, in email, or at either of the presentations -- seemed interested. So I deemed it best to let it lie. Seemed a better use of my time to write code that was wanted, rather than spend it on code that wasn't going to be used.
I could bring it up to current code base, but it probably wouldn't be any time soon as I'm rather busy at the moment. Would probably not happen before December, might even be after Christmas. If that timeframe doesn't suit, feel free to fork my mod_random_image repo and do it yourself; you have my blessing to take it and run with it as far as you can. While I'm fairly sure that it, specifically, is not necessarily the Right Way to do CMS extensions, I'm fairly certain the seeds of it are there, they just need some iterating over to bring it out.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-03-07 22:33:37 |
| Closed_By | ⇒ | roland-d |
Thank you for your contribution. We have discussed this within the PLT and decided that it is something we won't include in the Joomla 3.x series. However we would like to revisit this for Joomla 4.x and see if it can be considered for inclusion then.
Thanks for coding this, Arlen!
While we’re transitioning to a new integrated tracker, could you report the issue on our current main tracker at JoomlaCode and cross-reference each with a link to the other? Here’s the process for reporting on the other tracker:
http://docs.joomla.org/Filing_bugs_and_issues
Alternatively, let me know if you’d like me to create it for you and I can go ahead and do that.
Thanks in advance and thanks again for coding this, Arlen!