I'm looking over your code example, and while I don't think your framework would meet my own requirements as effectively as my own, I htink you should certainly open source it. It does solve many problems faced by many developers.
There is little point in my just saying why your framework wouldn't work for me, so I will try and draw on examples of where your framework would cause more trouble for me than my own solution. If you/we gain anything, hopefully it'll be indications as to what you need to document in order to get new developers up and running sooner.
Templates are a great place to start.
Someone just coming into your framework (like myself) would be confused by the use of the various functions used inside the template.
adef_tdata
I found it located at 'includes/core/functions.php'
Code: Select all
function adef_tdata($key)
{
$_KERNEL = kernel::get_instance();
return $_KERNEL->page_data->get($key);
}
It's a wrapper, and now I have to find the class for KERNEL. I find KERNEL and look at the class expecting to find a object called page_data but I see nothing. From quickly looking at KERNEL I can see that it loads and caches classes automagically and that page_data is is not a statically declared member variable.
From here, I do a search for the class 'page_data' and can now begin to figure out what that template function did.
Looking at page_data I'm slightly confused as to how that fits into the template.
After some poking around I see that index.php is invoked through mod_rewrite and that the whole process is kicked off by a call to:
Code: Select all
$_KERNEL->webpage->request(get('ADEF_GET_RESOURCE'));
Confused by what appears to be a constant being passed in I go looking for the define and a few minutes later realize it's actually a GET variable capitalized. Personally I am extremely strict about conventions and standards, I never capitalize anything but constants and thus consider that bad practice. Although you probably did that to avoid possible name clashes with custom/user GPC variables this is still bad practice IMHO. Besides what if someone designs a GET form with a field name labeled ADEF_GET_RESOURCE?
Now I'm looking into compiler->build()
Again, I'm strict about conventions and because of that, your code confuses me.
In class webpage the request() method is passed a $webpage variable which is passed to compiler->build() but the implementation uses the variable name $request.
While I aagree that some times functional context sets the name of a variable, in this case, I think I would stick to $request in both methods. It's just easier to follow when just entering the codebase and stepping through it trying to learn everything.
Your compiler class is to complex for my use. I can't even offer suggestions as to how to refactor it, cause I lack a solid understanding of the rest of the framework, but that is way to many lines. In that one file, you have the same funcitonality as I do in my:
1) Request object
2) Response object
3) View object
4) Template object
5) Caching object
6) Controller object
It's to much going on in one place, that without a long period of time spent learning how the code works as well as learning how it affects the rest of the code base, makes for an unplesant learning experience. Honestly I would be afraid to change anything because of the heavy dependency on $_KERNEL and it's objects.
That is where I stopped trying to learn your framework internals, unfortunately that seemed to be the heart of the framework right there, as whatever it returns is sent to the browser.
The implementation of a framework is of little interest to the average developer, so lets' skip anything above you find non-constructive and get back to the example you shown.
Templates:
Your template is typical, same technique as my own, using PHP. Great move.
Problem:
How does that adef_tdata turn out the properly formatted META tags? Makes me think that:
1) The HTML is hardcoded somewhere in a function - yuck
2) The script author is responsible for adding complete HTML meta tags - double yuck
Same concern applies to the navigation_menu. Where is the HTML required to create them? If I need to change the menu to include small icons next to the next or I want to remove the <a> link and use javascript instead, how do I do that??? The HTML is not in the template where I would expect to find it and following the code is difficult and time consuming because of the dependencies and amount of indirection when travelling your codebase.
Now I"ll jump on your actual scripts.
index.php has several aspects that would make me cringe if I had to work with it.
1) You have a SQL query in the same file as your HTML template. I cannot state enough how much I hate that and how it results in buggy code.
2) You have inline SQL, I equally hate that. When people develop software this way, in any application beyond something trivial, you get major code duplication and when one section of an application changes the other queries break, etc, etc.
Of course there is nothing stopping someone from using a model object to centralize SQL and such, but there is nothing influencing or suggesting I should either and people are lazy and will take the easy way out.
What is the status error in the templates all about?
I don't like seeing anything but presentation logic in my templates. Strict discipline required, but it keeps everything clean.
In my framework, setting an error message happens in my controllers and no where else.
edit.php looks like a typical transaction script. You have your basic SQL queries, conditional logic testing for whether you should save or not and finally you dump the results to screen via a template.