@pestaa: Let’s use another example. Assume class A has a read-only property called parent. This property can only be set via constructor, i.e., new A($parent). Now what would you do with configuration as parameter? Of course, there are still some workarounds. But mostly, they are not straightforward. I am using this example mainly to show it is a strong requirement to ask for __construct($config).
I don’t quite understand your last question.
But I’m not saying move the init() stuff to the constructor. Doing so without due consideration would be silly. But in the cases I looked at, there was no problem doing so. Please give an example, of the above cases, where this wouldn’t work.
As I showed, I’m saying do what it necessary in the constructor – and certainly in a lot of the cases above the init() stuff can go there – and then explicitly do whatever else is required – because that makes the code clear and easier to comprehend. At the moment things are obfuscated and whether or not to do something, and where, seemingly random. The cause seems to be this init() thing.
No, I’m not suggesting building huge constructors either. That is counter to everything I have said. I’m saying “get this stuff out of here and put it where it belongs”.
Anyway, as I say in my reply to pestaa, you can replace constructors with factory methods, and in complex cases – and there don’t appear to be many of them – that’s a good solution, imo.
So what is your suggestion? I have yet to see any concrete one yet.
Do you mean merging init() into the constructor for application component? As I already said, this means the constructor then must use the signature that pestaa suggested, which is too strong. Theoretically, any component can be an application component, as long as it implements IApplicationComponent. So asking for such strict constructor means this interface is incomplete.
I don’t think factory has anything to do here, and I don’t think using a lot of factory methods is a good thing. Factory itself has a lot of limitations.
http://php.net/manual/en/language.oop5.interfaces.php#85859
The documentation does not state anything against this statement.
They can be implemented inappropriately, of course, but I don’t think they are restrictive by definition.
Thanks for the pointer about constructor. I didn’t know this (it wasn’t allowed before). In general, I’m against posing a requirement on the constructor signature because it doesn’t follow the spirit of interface.
I don’t mean factory is useless. Factory is not good when subclassing is needed.
I think we should stop discussing on this. I realized this is more like personal preference, and it is hard for one to convince the other. As long as we use init() in a consistent way, I don’t see big problems.
It’s a shame, I was hoping to contribute to Yii both with the documentation and the code.
But you don’t! As I demonstrated. And there seems to be no way of persuading you about anything once you have you mind set, which is kind of limiting.
Many FOSS projects work well with a closed approach to development, and I’m happy to use them, but I’ve found it’s not fun to try to work that way. Different strokes.
I’ll still use Yii, I’ll just remove the obvious flaws and procedural code.
Right! I’m off to hack ActiveRecord now I’ve got init() out of the way (mostly).
I am open to your suggestions. But, please, give some concrete ones. Having typed in so many words and spent so much time, I have yet to see any from you yet (pestaa gave one, but it’s not convincing enough to make the change). That’s why I call for stopping discussing on this.
I did a Google search to find out factory drawbacks, but did not succeed. Please invest a few minutes to clarify what you meant by ‘not good’.
I’m willing to accept your decision without further questions if it is well reasoned. However, I can’t see any purpose why we should keep the existing instantiation process without considering alternatives.
If a factory method not only creates a new class but also configures it from an argument, you then have the ability to customize the configuration process. It also means that external classes know less about the components’ internals, which is always good, even if subclassing a component takes a bit longer.
Here’s an example of what I thought:
class CComponent
{
protected function __construct()
{
// pre-initialization
}
public static function factory($config=null)
{
$class = new self;
// do whatever configuration needs
// a foreach might do so far
// post init, or init as in current implementation
return $class;
}
}
Edit:
The only drawback I found (bug 30934):
Using the factory method, how would you write a child class that needs to do some initialization before parent’s initialization?
The "self" bug you mentioned is a big one. This can only be solved in PHP 5.3.
I needed to investigate first. Then experiment. I don’t know your code, and there are no tests, so it takes a lot more time than a well organised project. So I have to create tests, branch, and validate my ideas. That’s why I asked questions, to speed things along. I have no wish to rock the boat.
No problem. There are some unit tests under "tests" folder. They only cover part of the code. Adding more coverage of tests is one of the major tasks looking forward (mainly need to find good ways to set up different fixtures).
I apologize if my posts hurt you. I guess I am overheated.
This framework is far from perfect. There are many places that need to be improved. One of the biggest obstacles to these improvements is to maintain backward compatibility (BC). For this reason, I have to defend hard when a design change may break BC.
Having said that, I don’t mean we should sit on mistakes. We may take your suggestions when we do a major release or rewrite that doesn’t have keeping BC as top priority.
By extending constructor and calling parent implementation lastly.
Update:
Backward compatibility is definitely an important point of view, but as these ideas are really long term, I think they simply cannot be implemented before the next major version. But it’s fine.
Could you write some pseudo code to show this? Note that these initialization (both parent’s and child’s) need to be done after configuration.
Of course. Please note that prior to 5.3, PHP forces me to copy self-referential methods, which might turn the whole thing even more harmful than the current implementation. I can’t decide, though. I’m also not aware the possible workaround you mentioned regarding 5.3.
class CParent
{
protected function __construct()
{
// before config
}
public function factory($config=null)
{
$class = new self;
// config
// ...
return $class;
}
}
class CChild extends CParent
{
protected function __construct()
{
// before parent init
parent::__construct();
// after parent init
}
public function factory($config=null)
{
$class=new self;
// config
// ...
return $class;
}
}
Subclasses of CActiveRecord are required to have a model function to overcome this self issue. I know this is overwhelming.
If configuration is complex, we can dedicate a protected method for it. Otherwise it can be done in the constructor (which won’t make that ‘a big black box’). So only factory($config=null){return new self($config);} would be necessary in each descendant.
Nope, your code doesn’t meet the requirement. I was saying the init code is needed after the configuration is applied (e.g. establishing DB connection after the connection parameters are set).
I’m not at all offended. No pasa de nada.
Sure. Completely understood.
BC (and regresion, of course) was also one of the issues I was considering, which is why I needed good test coverage of the areas I was looking at. That’s not a ten minute job!
No problem,
w000000t, I was so happy ready this, and all of the sudden it stops? I deserve a conclusion!