Jump to content

why is ConfigurableModule::getModuleConfigInputfields() declared as static?


mindplay.dk
 Share

Recommended Posts

Like the topic says, why?

It's causing me all sorts of pain - because the module configuration callback has to be declared as static, getting to properties inside the module is difficult, so I ended up switching a lot of instance-variables to static, so that I could get to them, which causes even more mess and confusion.

It appears that, by the time the static method is called, the module has already been instantiated anyway - so what reason is there for this to be static?

Really frustrating...

Link to comment
Share on other sites

It's static so that all configurable modules can share a common interface. Some modules do stuff when they are instantiated, like hook things, queue CSS or JS files, or other things that aren't related to configuring a module (and may even be a problem if present when configuring a module). So we need the ability to know what it takes to configure a module without actually instantiating it. The only module type that would be instantiated at the same time as configuration would be an autoload module.

If you are dealing with an autoload module, or you are willing to manage your own configurable vs. executable state, then there is an easy pattern you can follow. This assumes you have two configuration variables in your class, 'foo' and 'bar'. 

class MyFooBar implements Module, ConfigurableModule {

  public static function getModuleInfo() { /* you know what's here */ }

  public function __construct() {
    // set your default values for config variables
    $this->set('foo', ''); 
    $this->set('bar', 0); 
  }

  public function init() {
    // note that PW populates the actual values for your config vars, 'foo' and 'bar'
    // before this init() function is called, but after __construct is called. That's why 
    // we set the defaults in __construct. Once you get to this init(), your config is 
    // now populated, and likewise available for this non-static function below: 
  }
  
  public function getConfig() {
    // you can name this function whatever you want, since it's not part of any interface
    // this is the stuff you would usually have in your static getModuleConfigInputfields
    // see how we can pull the values from $this->foo rather than $data['foo']; 
    $inputfields = new InputfieldWrapper();

    $f = $this->modules->get('InputfieldText');
    $f->attr('name', 'foo');
    $f->attr('value', $this->foo);
    $f->label = 'Foo';
    $inputfields->add($f);

    $f = $this->modules->get('InputfieldInteger');
    $f->attr('name', 'bar');
    $f->attr('value', $this->bar); 
    $f->label = 'Bar';
    $inputfields->add($f);

    return $inputfields;
  }

  public static function getModuleConfigInputfields(array $data) {
    // note we don't actually need $data in this pattern
    $module = wire('modules')->get('MyFooBar'); 
    return $module->getConfig();
  }
}
  • Like 2
Link to comment
Share on other sites

  public static function getModuleConfigInputfields(array $data) {
    // note we don't actually need $data in this pattern
    $module = wire('modules')->get('MyFooBar'); 
    return $module->getConfig();
  }

This is precisely what I ended up doing - but according to what you just said, that could cause problems, since I'm now forcing the module to load at configuration-time. (?)

Link to comment
Share on other sites

This is precisely what I ended up doing - but according to what you just said, that could cause problems, since I'm now forcing the module to load at configuration-time. (?)

Could cause problems with modules that are loading other modules or populating JS/CSS files to $config->styles or $config->scripts. If your module is already an 'autoload' module, then it should be no problem since it's already loaded and obviously not a problem. But this is more of a consideration with Process, Inputfield and Jquery modules, that are loaded on-demand, as they do trigger loading of other assets. And you don't want those assets loaded unless the context is right for them to be loaded. Ultimately it just depends on what your module is doing. Since PW doesn't know what all the modules are doing, we needed a universally safe way to configure all modules, and a static method gives us that. But that doesn't mean that you have to stay static when you don't need to. 

Link to comment
Share on other sites

Whenever I see the static keyword, I cringe, and I automatically start scrutinizing my code to try to understand why I'm using it - the more I do that, the more I find that investigating your own motives for using statics usually (if not always) reveals a design flaw.

It sounds to me like the Module concept really has several different things sort of huddled into one class. The fact that the class itself has a life-cycle that begins before it's even constructed, is one indicator - the class itself has an area of responsibility that begins before it even exists. Another indicator is the configurable options that affect the object's life-cycle - a static/singleton module tends to work more like an application-wide, shared service component of some kind, while other modules such as Fields are accessed through the same container (wire('modules')->xyz) where now the container suddenly works more like a factory than a service-broker.

All of these differences could be modeled more explicitly using interfaces and polymorphism, as opposed to static methods and configurable options that affect  the behavior of framework components and life-cycles of module objects.

I would suggest rethinking this architecture for a future major release.

 

For example, a module, conceptually, is a singleton - it consists of one set of files implementing classes, both of which are by definition singletons, hence, every module is going to have one singleton object providing information/metadata about the module itself. It would be natural to refer to the base-class for that object as the "Module". By making that it's only responsibility, you can eliminate the need for statics, and the Module class itself becomes much simpler and more lightweight.

Modules that implement a module-wide configuration screen could provide the configuration model/form via an interface, e.g. a ConfigurableModule interface with a getModuleConfiguration() method, returning a dedicated configuration object of a type that implements an abstract ModuleConfiguration base-class. This eliminates the need for those static methods, and enables better composition, e.g. the freedom to extend different configuration base-classes for more specific purposes, etc. - it also isolates the responsibility of configuration and the configuration-form, enabling the code to load independently of the module itself.

Similarly, modules that implement InputFields, which are not singletons, could implement another interface, e.g. FactoryModule, with a method createInstance() that returns an InputField or something else with a non-singleton lifecycle. This provides more separation and eliminates the need to load the instance-types before they're needed - and those instance-types can now deal exclusively with their area of responsibility. This also clearly separates service singletons from component instances, and gives those types a distinct lifecycle - by introducing the same separation in the framework API, this becomes more obvious to the consumer as well, e.g. wire('modules')->FancyInput refers to the FancyInput Module singleton, whereas e.g. wire('factory')->create('FancyInput') clearly is expected to create a new instance of a component provided by the module.

And so on along those lines.

I hope you're not opposed to making architectural changes in major releases in the future? Many architectures of this type (Drupal and WordPress to name two) are set in stone, and even major releases don't tend to break backwards compatibility, but nothing ever really improves. Drupal was actually nice when it came out, it had some good ideas at the core - but time ran away from it, because the selling point "lots of modules" is hard to argue with, and breaking backwards compatibility (in a major way) can feel like starting over. In my opinion, if you make substantial improvements to an architecture, it's worth breaking backwards compatibility. We should not reject new ideas because they're not compatible with our current world view or legacy components - if we do, we can't make never make any real progress.

Anyways, </rant> :-)

  • Like 1
Link to comment
Share on other sites

Whenever I see the static keyword, I cringe, and I automatically start scrutinizing my code to try to understand why I'm using it - the more I do that, the more I find that investigating your own motives for using statics usually (if not always) reveals a design flaw.

I'm guessing you don't like Laravel much? :)

There is a reason why statics exist as a language construct, and there is a reason why we use them where we do. Though our usage is admittedly rare, there has never been a goal to "avoid statics as much as possible". The goal has been to use the tools available to us to make ProcessWire as simple, flexible and extendable, to the intended audience, as possible. It's not often that we have use for a static method in ProcessWire, but when we use them it's because they are the most appropriate solution. Keep in mind, we don't actually "need" static methods. We could certainly do without them. They just make a whole lot more sense in our context than the alternative. 

It's important to realize that Module is just an interface for communication between ProcessWire and the functionality you want to provide. It is not a class or even an abstract class. It provides no implementation for you (other than what you might optionally choose to extend from some other class). The only requirement for a module is that ProcessWire can ask it what it is. That comes from the Module interface's 1 static method: getModuleInfo(). Without that, it is not a Module. The ConfigurableModule interface has 1 static method: getModuleConfigInputfields(), which you can choose to delegate elsewhere if you choose. Beyond the obvious benefits, these methods are static for correctness: they are about all instances, not a specific one.

The existence of the interface is not a suggestion that you implement everything in the class itself. That's entirely up to you. If the scope or philosophy of your need is such that you want to split every part of it into separate classes and files, then you should (this is what I do in FormBuilder). The Module interface facilities this. But the reality is that most modules are not of that scope, and there's rarely a tangible benefit in our context to being more verbose. But we ultimately leave that choice to the developer. 

For example, a module, conceptually, is a singleton - it consists of one set of files implementing classes, both of which are by definition singletons, hence, every module is going to have one singleton object providing information/metadata about the module itself.

A module is a singleton only if the developer specifies "singular" in his/her module definition. Otherwise you will get a new instance every time you ask for a module.

Modules that implement a module-wide configuration screen could provide the configuration model/form via an interface, e.g. a ConfigurableModule interface with a getModuleConfiguration() method, returning a dedicated configuration object of a type that implements an abstract ModuleConfiguration base-class. This eliminates the need for those static methods, and enables better composition, e.g. the freedom to extend different configuration base-classes for more specific purposes, etc. - it also isolates the responsibility of configuration and the configuration-form, enabling the code to load independently of the module itself.

I understand what you are trying to get at here. There may be someday when the configuration needs of modules increases in scope to the point where we might benefit from such an approach, but we're not near that yet. In the present, I think the majority of module cases benefit more from less verbosity and the current approach. We already have the door open to this approach, even if it's not implicit--FormBuilder uses something very similar, for example. But I would be happy to support this more implicitly as a second option for module configuration in the future. 

This provides more separation and eliminates the need to load the instance-types before they're needed

Instance types are not loaded before they are needed. They are loaded on-demand, unless the module's definition specifies "autoload". ProcessWire caches its module information so that it doesn't need to even include a module's file until ready to instantiate. 

This also clearly separates service singletons from component instances, and gives those types a distinct lifecycle - by introducing the same separation in the framework API, this becomes more obvious to the consumer as well, e.g. wire('modules')->FancyInput refers to the FancyInput Module singleton, whereas e.g. wire('factory')->create('FancyInput') clearly is expected to create a new instance of a component provided by the module.

It's the module developer that should make the call about whether their module is designed to be singular or multi-instance. A singular module might very well be coded differently than a multi-instance one. I don't want the consumer to have to think about this variable. 

I hope you're not opposed to making architectural changes in major releases in the future?

I'm not opposed to making architectural changes in major releases so long as they are geared at make things simpler or easier for the users of the software. While I don't share all your opinions on how some things should work, I appreciate and respect them, and am especially glad for your interest in them. If we were to implement an architectural change to make module configuration more implicit to a separate class, I'd support it (after all, it's an approach I already take in some modules). But it would be added as an option, rather than a replacement. In terms of future major releases, I don't like breaking backwards compatibility unless absolutely necessary. But if there's a net benefit to the wider audience, then I have no problem with it. The only thing in my mind that carries that status right now is the switch to namespaces in 2.4 (and the changes that would go along with it), which I'm looking forward to collaborating on.

  • Like 3
Link to comment
Share on other sites

I don't want to get into a long drawn-out discussion, but suffice to say, some of your answers simply explain how things currently work, which I already know - if you'd like, you can read my answers again and assume that I already know what you told me, maybe you'll extract something new from my comments.

I'll just briefly comment on two things.

A module is a singleton only if the developer specifies "singular" in his/her module definition. Otherwise you will get a new instance every time you ask for a module.

...

It's the module developer that should make the call about whether their module is designed to be singular or multi-instance. A singular module might very well be coded differently than a multi-instance one. I don't want the consumer to have to think about this variable.

But the consumer does have to think about this variable - it's crucial to know when you ask for wire('modules')->xyz whether that returns a shared instance or a new instance. My point is that these are two entirely different life-cycles, and that fact is not expressed by the API in any way. The module container doubles as a factory-class - it provides services as well as components, with no obvious distinction in the API. That doesn't seem right to me.

I'm guessing you don't like Laravel much?  :)

Actually, you're spot on - I was excited about Laravel at first, but then I looked under the covers... I'm already suffering with the limited extensibility of a framework that uses statics (Yii) and I find it crippling when I want to override or extend core functionality. Code that consumes a static interface is hog-tied to the class - meaning you can never extend, override or replace that class, because the class-name itself is referenced in all the consumer-code. It makes my brain hurt. It really does.

Frameworks (and preferably all code) need to be completely open to extension - statics get in the way of that, in a huge way. Yii is bad enough - Laravel would probably make me want to kill people.

Simply put, things like Database::getConnection() is fine until you want to override the getConnection() method and you realize that all the consumer code is filled with static references to the Database class. Where does your extended class fit it? It doesn't. So you're left to either hack the Database class, fork the framework, override a class-path in the autoloader, or some other horrible hack.

Statics in general: no thank you.

The only time you should use the word "static" is internally in a class that, say, caches something. A consumer API should never, ever use static interfaces...

Link to comment
Share on other sites

I don't want to get into a long drawn-out discussion, but suffice to say, some of your answers simply explain how things currently work, which I already know - if you'd like, you can read my answers again and assume that I already know what you told me, maybe you'll extract something new from my comments.

What I've extracted is that you don't like statics. You've set a goal of avoiding statics at all costs. And that's fine--I understand your reasons. I'm middle of the road on statics--I avoid them, but not at all costs. There is a reason they don't appear in PW's public API. But if they are the best fit for a given situation, given all factors, then I'd consider it a bad choice to use something else. We've got very good reasons for using them where we do. I hope you can understand why we wouldn't abandon an interface that works very well for us just because one person doesn't like statics.

What I wrote in my previous message was that I support your ideas, but as an alternative method of configuration, not as a replacement. Step out of the ivory tower and propose something tangible and realistic. Use some code. For instance, I understand you'd like to have a separate class to handle configuration. I like this idea for many situations. So how would you formalize that in code? What specific methods would be in the ModuleConfig interface or abstract base class? How would you propose relating the configuration class to the module class? I'd suggest that it could be done by an agreed-upon class name, like "Config[ModuleName] implements ModuleConfig", or a module's getModuleInfo() could specify it's configuration class in it's info array. But maybe there's more ideas too. Get some momentum going here and maybe we'll have something we can commit to the code base now rather than later. 

But the consumer does have to think about this variable - it's crucial to know when you ask for wire('modules')->xyz whether that returns a shared instance or a new instance. My point is that these are two entirely different life-cycles, and that fact is not expressed by the API in any way.

I understand why it matters in many situations. But this distinction does not matter in ProcessWire modules. Can you think of any instance where it does? We go out of our way to keep these considerations away from those actually using the module.

Actually, you're spot on - I was excited about Laravel at first, but then I looked under the covers...

I don't yet know much about Laravel, so can't knock it or praise it. But I was turned off by the syntax I was seeing in the examples. Though I can find plenty to like after a quick look too. But it does seem like a lot of people are treating Laravel as God's greatest gift to PHP. So far I don't understand. Maybe there's something to learn here, even if there is some to dislike as well.

The only time you should use the word "static" is internally in a class that, say, caches something. A consumer API should never, ever use static interfaces...

While I don't hate statics like you do, I do agree in general -- I don't much like seeing them in APIs. That's why you don't seem them in ProcessWire's API

Link to comment
Share on other sites

My issue isn't primarily with the use of statics - there's a right time for statics, but there are far more cases where they're wrong.

My main issue is separation of concerns - when a class has too many responsibilities, inheritance doesn't "work", and object lifecycles become too complex. Classes with responsibilities that switch or change over time is one indicator. Multiple stages of initialization, e.g. __construct(), init(), ready() is another indicator.

To use a simple example, let's say the class X_Y_Z has three responsibilities - assuming it's not doing three things at once, it's usually better to break this into 3 classes X, Y and Z each with a single responsibility, and then introduce a composite class A with properties $x, $y and $z... the problem with class X_Y_Z is that, if you extend that class, your extension needs to deal with 3 areas of responsibility, even if your extension actually only affects one or two of those responsibilities.

The higher number of classes actually reduces complexity by allowing more granular composition. If X, Y or Z have different lifecycles, as is likely if they have different responsibilities, their individual lifecycles can consist more naturally of __construct() and __destruct() as is the idea with OOP - partial initialization (initialization in stages) happens in X_Y_Z because it has different responsibilities at different times, which I find to be an indication that it's time to refactor.

Of course, once you have enough dependencies on something, refactoring becomes increasingly costly - and in the case of a modular CMS like PW, this impacts a lot more people, and I'm not advocating a big change right now. I certainly don't have the time to propose and entirely new API right now, too many other things going on. Just trying to provide some architectural feedback, to improve things for developers in the future.

It's not even unthinkable, like you suggest, that an improved API could remain backwards compatible. I wish I had more time to contribute to this, but we're planning a move this summer, so I've really got too much on my mind right now. Sorry I can't provide more concrete suggestions.

Link to comment
Share on other sites

I think PW has the best module system in place I have ever seen. I haven't seen even a single example where it in real life usage falls short (ie. actually prevents or slows down module development). Or any example where getting rid of that static would make it much better?

Link to comment
Share on other sites

@mindplay.dk: Separation of concerns, single responsibility principle, and so on -- I get it. You seem to think we're designing these things without understanding of these principles, and that couldn't be further from the truth. I'll also assume that you understand what I was stating before, that we've tried to adopt enough flexibility so that a developer can choose to adopt these principles, while still using the module interface. But we don't want to enforce that upon everyone because it would be counterproductive design.

Why? There are principles, context, audience and goals. Our context is not the same as that of the Symphony framework, for example. Our audience has crossover, but is not the same. Most module cases are relatively tiny components. We have a project goal of making module development as simple and bulletproof as possible... something that might encourage web developers to take an interest in exploring their own module development. Part of that is an original design requirement of being able to share and communicate simple modules as 1 file / 1 class, when one chooses to (as many do). The ability to be super-simple and self contained like this encourages growth for the entire ProcessWire module ecosystem. We derive a lot of benefit from this, and that was the goal.

It's not at the expense of the underlying principles unless you choose to make that sacrifice yourself. Don't develop your big module using the same strategy as your small module. We don't have many implied strategies for big modules and what's probably lacking most is just the documentation to explain when and why you might take one approach over another.

Don't think I'm making any claims of perfection with any bit of code or design. There is always room for improvement in anything, in any project, in any system, in any file, in any class, in any method, and so on. The definition of a bad coder is someone that thinks they've got it all figured out or something is beyond improvement. I am very far from that. As most here know, I'll defend that which is worth defending and agree on things that aren't. But there's a lot more thinking behind the design than you often assume. I feel strongly about the decisions behind our current module system, and that they were the right ones. I also have an open mind. If someone proposes something better that understands not just the principles, but the context, audience and goals too, then I welcome it. 

  • Like 2
Link to comment
Share on other sites

  • 4 weeks later...

Hello, I'm new to ProcessWire but i like what i see so far.

On the CMS side, I'm a forced worpdress dev and a Modx enthousiasth, but I feel alone lately as Modx seems to not be as exciting as it used to be (for me).

I don't yet know much about Laravel, so can't knock it or praise it. But I was turned off by the syntax I was seeing in the examples. Though I can find plenty to like after a quick look too. But it does seem like a lot of people are treating Laravel as God's greatest gift to PHP. So far I don't understand. Maybe there's something to learn here, even if there is some to dislike as well.

I've made website with Laravel and it's a big gift to the php framework's world.

Laravel 4 (which is coming in may) will use the __callStatic() provided by php.

By using a nice dependency injection (IoC) container combined with a nice Facade interface, Laravel allows developpers to use a very expressive syntax while keeping the advantage of public method.

In Laravel 4, when you type View::make() underneath View->make() is called.

This setup also allows the code to be tested properly while it's "complicated" with static methods.

I'm not sure that will help you but I've played extensively with it and i find it awesome in many ways (to the point of making a similar approach to get rid of globals in my... WP hooks hell).

That being said, i've tons of reading and testing to do with a new toy called ProcessWire :)

  • Like 3
Link to comment
Share on other sites

That being said, i've tons of reading and testing to do with a new toy called ProcessWire :)

You will be surprised at how quickly things are going to click for you - everything is so simple and intuitive, it basically just makes sense, without much reading or experimenting. Very different experience from any other CMS I've tried  :)

  • Like 3
Link to comment
Share on other sites

Hello, I'm new to ProcessWire but i like what i see so far.

On the CMS side, I'm a forced worpdress dev and a Modx enthousiasth, but I feel alone lately as Modx seems to not be as exciting as it used to be (for me).

Hi Lossendae! Welcome and nice to see you here! You'll find a couple of former MODxers right here (including me, of course). I agree with mindplay.dk..you will be surprised, shocked even at how easy and fast you can accomplish tasks in ProcessWire. It is so intuitive and the API, oh my the API...let me not spoil the surprise in store for you...but PW will blow your socks off!

Welcome!

/k

  • Like 2
Link to comment
Share on other sites

Forced WordPress dev and MODx enthusiast sounds like my bio from a few years back too :)

Welcome to the wonderful world of ProcessWire. You may find as I did that you have to un-learn old stuff as much as learning new stuff and the rule to work by with ProcessWire is "if it seems too difficult then there's probably an easier way" which is what we're all here for on the forums ;)

  • Like 1
Link to comment
Share on other sites

  • 1 month later...

Hello Everyone,

I somehow missed this discussion until now...

I've built sites with Laravel, and before that with CodeIgniter. Then I discovered ProcessWire. Laravel is an amazing framework. However, after close scrutiny I say ProcessWire has the potential to be even better. Already, ProcessWire allows deep and flexible functionality that takes extra work in Laravel. The syntax of ProcessWire is even more "expressive" than Laravel for a majority of needs. With ProcessWire, we achieve that tricky balance of having the essential core management/development powers of a CMS, and yet maintaining the flexibility of a framework -- which is how ProcessWire earns the CMF title. With many other systems, you either have too many core assumptions (the Big Three), or too many core functions must be done repetitively (frameworks).

Now, I can pinpoint a few isolated areas where Laravel has powers not yet in ProcessWire. But one by one I find ways to take care of them. For example, I had wondered about the need for a forms library in ProcessWire, but all it takes is the inclusion of a library like Zebra forms and we have it. Actually, it ends up even better!

Still, I think there is good reason to compare ProcessWire to Laravel. That way, the last few remaining spots might be addressed. I already prefer to build all my new projects with ProcessWire. There is no doubt in my mind that if ProcessWire continues along the path it is on, there won't be any advantages to using Laravel.

Thanks,

Matthew

Link to comment
Share on other sites

there's a lot more thinking behind the design than you often assume.  

Just re-read your message, and just wanted to make this clear: I do not make assumptions about your design.

When I ask questions, challenge your ideas, or propose alternatives, I'm merely testing the boat to see if it holds water - it's nothing personal, it's just the way my mind works. And most of the time, you are able to clarify your objectives or explain your choices to my satisfaction :) ... as Einstein said, "question everything" - this is how we arrive, not at perfection, but as you say, the best possible choices gives the context and goals.

Please assume that I'm trying to help - if my tone suggests otherwise, I apologize, I'm not always eloquent with my words.

I never got on the Drupal, Joomla or WordPress forums to try to help there - because I never saw any of those projects as being worth my time. This project is. So I hope you'll put up with me, even if I ask obnoxious or provocative questions at times.

As Rafael Dohms just pointed out on Twitter: Discussion is evolution, whether you are right or wrong the exercise of discussing will teach you something new.

:)

  • Like 6
Link to comment
Share on other sites

Greetings,

mindplay.dk: I think Ryan and others here are exceptionally good with challenges!

I have the same mindset as you: I am always testing, and I believe challenges are good for ProcessWire.  I think Ryan definitely embraces healthy challenges!

I believe that when a system is as great as ProcessWire, it benefits from being compared to other terrific systems.  That's my intention when I bring up Laravel or other frameworks.  Like you, I realized very quickly that such challenges go nowhere with The Big Three systems.

The more I use ProcessWire, the more I see that it conquers major design/development territory.  Bringing up isolated challenges -- as long as it's done with the intent to build and improve -- can help ProcessWire conquer the remaining territory.

Thanks,

Matthew

Link to comment
Share on other sites

When I ask questions, challenge your ideas, or propose alternatives, I'm merely testing the boat to see if it holds water - it's nothing personal, it's just the way my mind works.

Don't worry, I understand. No need to explain, we're all on the same $page. :) Just trying to make sure I follow up good questions with good answers. 

Link to comment
Share on other sites

  • 1 year later...

I Used this somewhere,

Worked for me:

protected static $defaultSettings = array(
    'foo' => null,
    'bar' => '1',
);

public function __construct() {
    foreach(self::$defaultSettings as $key => $value) {
        $this->set($key, $value); 
    }
}

If the module extends at least WireData, you could also use setArray:

public function __construct() {
    $this->setArray(self::$defaultSettings);
}
  • Like 3
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...