Great feedback ... really appreciated. The framework needs a lot of work and input like this is very helpful.
Mordred wrote:I had a quick glance at the framework, and what I think it's major problem is, is an advanced form of OOP-ittis

Seriously, I don't think it's normal to have that amount of code split into that amount of files. Following a simple action requires opening of numerous files, where numerous objects delegate roles which noone would never want to change (so, there wasn't any need of that delegation in the first place). I don't have the code now so I can't give concrete examples at the moment, but the numbers kinda speak for themselves.
You are right, the framework is split into a lot of files. I don't think that one class per file is that unusual. Maybe others can comment on that. But the main reason for splitting everything up has been to support implementation flexibility. The goal is that you don't have to include stuff you don't want. Perhaps that is a poor goal, or we have done a poor job achieving it.
I would be really interested to know what you think should be combined. It is quite possible that classes could be combined. The splits may be leftover from previous design ideas that are not longer needed.
I checked the number of files that need to be included in a bare bones system -- it is 5 classes. That included a Request, Registry, Front Controller and Mapper, plus a class to hold class/method names for the default and error actions. That could certainly be reduced. We could make it so you do not need a Registry. The FC and Mapper could be combined. And the class/name holder class could be turned into an array. There are reasons for those classes to be split, mainly to support alternate configurations, but perhaps they should still be combined. It is worth discussing.
Another solution might be a alternate combined class that supported a standard mix of features. There is currently also a simpler Front Controller that does not require any other classes if you want to use that (added to download at
http://ap3.sourceforge.net/downloads/). Or you can just launch Actions as Page Controllers if you like.
I went back and checked a full featured bootstrap and it includes 15 files. That adds to the above list: View and Response classes, Router, Session, Config and Template classes. That sounds like a pretty standard mix. The reason for splitting things up is to allow programmers to use only the mix they want. Does that sound like too many files/classes to you? How many do you think is a good practice?
I am also interested to know if it is the number of classes or the number of files that is of concern. I think maybe it is the number of class because of your "OOP-ittis" remark. I would certainly like to eliminate any unnecessary delegation. I can provide some examples and the current rational if that would help you identify these problems.
Mordred wrote:I went to take a look at the sql layer, and I'm unimpressed. The fluent interface is a nice OOP gimmick, but not very suitable for the task. While I'm all for using an SQL generator instead of writing SQL code, in the end the PHP becomes LONGER than the corresponding SQL code, with little to no added benefit. I would expect a generator to ease the majority of work related to writing SQL code. The only benefit I see from this is that a PHP interface is still better than string concatenation, and that you could still write an upper layer on top of this one and work with SQL from a higher level. The separation of SQL code and DB driver is also ok, but not really called for (IMO, could be wrong. Any of the authors care to justify it?)
I generally agree with you that in most cases generator code is probably worse than plain SQL. The SQL classes you mention are completely optional however. There is also a Prepared Statement class available as another alternate way to generate SQL separate from the OO style classes you mention. None of these classes are required to make queries -- they just generate SQL strings that can be used with any database library. This is a good example of the loose coupling that we have tried to implement.
If you can think of improvement to the SQL generator's interface, please let us know. There is always room for improvement.
Mordred wrote:There is a security vulnerability in the way SQL parameters are escaped, which would allow quoteless SQL injection to be used. I'll leave it as an exercise for the reader, at least until I go home and paste the code

I would really like to know about this. The SQL classes are new and incomplete. We certainly want to fix this problem so if you can provide an example or patch to the code it would be greatly appreciated.
The framework is in an Alpha phase where the design is still fluid so all this input will have a real impact on the code. I hope you can become a little more familiar with the framework and give us deeper design input. Thanks ... and keep the criticisms coming!
