Here guys - adeframework.com
Moderator: General Moderators
Here guys - adeframework.com
Tear it apart.
http://adeframework.com
This is sooo not ready to be looked at. I threw that site up in about 3 hours.
http://adeframework.com
This is sooo not ready to be looked at. I threw that site up in about 3 hours.
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Here guys - adeframework.com
What is the license? What does "not yet public domain" mean?
(#10850)
Re: Here guys - adeframework.com
It means there is no license because I haven't decided to make it public domain yet or not. I guess it doesn't matter if nobody likes it though;) The code base is available for peer review.
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Here guys - adeframework.com
Well if there is no license then it is proprietary, which makes it difficult to look at.
(#10850)
Re: Here guys - adeframework.com
arborint, honestly I value your opinion, but I'm not going to license it just so people can look at it. If people are too scared to look at it fearing a lawsuit that is their choice. Based on everything you have said so far to me today however, I really don't think you would have anything nice to say anyway. It would be nice if you could focus on the positive and how to improve it rather than identify everything that you feel is absolutely horrible. I don't know if you're just a grumpy old man, having a bad day, or if you just don't like me, but it's not nice.
I know that the code is far from perfect, but I really like it, and I'm not going to suddenly dislike it and throw it away because you say it's bad.
I know that the code is far from perfect, but I really like it, and I'm not going to suddenly dislike it and throw it away because you say it's bad.
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Here guys - adeframework.com
Much as you might want to make is about feelings and attitude, it is really about simple professionalism. You are asking people to contribute to something that belongs to you -- for free. You will benefit from improvements, but you decline to state a license. It would be trivially easy for you to select a license. Why not do so to make it clear and easy for people to look at the code.
(#10850)
Re: Here guys - adeframework.com
I'm just showing some cool code to others, gathering opinions. Seems to me that is the whole point of the devnet site. Anyway, I look forward to any constructive criticism and comments from others who are interested.
Re: Here guys - adeframework.com
Hey guys, why can't all just get along nicely?
Anyway, I downloaded the framework and had a look. I have to say, it has some interesting concepts and obviously I can see how it can save you (its developer) development time. It is actually somewhat similar to how an OS works.
However, for using OO syntax it fails to capitalize on several things that make OO useful. Basically, it is completely procedural code in OO clothing - which there is nothing wrong with, it's just that OO can give you so much more in terms of code-reuse, structure and modularity, maintainability and extendability.
First of all, all the classes (that I've looked at it) go only one deep, meaning there is no class inheritance structure to speak of. For example, looking at the mysql db (dbmysql and dbmysqli) classes, they share many similar functions (some completely identical) and both could have inherited from the same parent class (reusing much of the code).
Furthermore, all the classes rely heavily on the Kernel class to work, setting up deep dependencies inside the framework (meaning they can not work independently). The heavy use of the Kernel and magical functions (__get(), __set()) also obscures the chain of command and makes it hard to track down where a method is actually defined.
You have some PHP4 conventions in there that aren't considered good practices anymore, such as the use of define's and global's. Your Kernel class is a singleton implementation, why not use its static getInstance() method instead of using global?
Having said all of this, I found your code to be very readable and consistent in coding style (though I prefer a different style, but that's another matter). Too bad there is no inline documentation.
Anyway, I downloaded the framework and had a look. I have to say, it has some interesting concepts and obviously I can see how it can save you (its developer) development time. It is actually somewhat similar to how an OS works.
However, for using OO syntax it fails to capitalize on several things that make OO useful. Basically, it is completely procedural code in OO clothing - which there is nothing wrong with, it's just that OO can give you so much more in terms of code-reuse, structure and modularity, maintainability and extendability.
First of all, all the classes (that I've looked at it) go only one deep, meaning there is no class inheritance structure to speak of. For example, looking at the mysql db (dbmysql and dbmysqli) classes, they share many similar functions (some completely identical) and both could have inherited from the same parent class (reusing much of the code).
Furthermore, all the classes rely heavily on the Kernel class to work, setting up deep dependencies inside the framework (meaning they can not work independently). The heavy use of the Kernel and magical functions (__get(), __set()) also obscures the chain of command and makes it hard to track down where a method is actually defined.
You have some PHP4 conventions in there that aren't considered good practices anymore, such as the use of define's and global's. Your Kernel class is a singleton implementation, why not use its static getInstance() method instead of using global?
Having said all of this, I found your code to be very readable and consistent in coding style (though I prefer a different style, but that's another matter). Too bad there is no inline documentation.
Re: Here guys - adeframework.com
Good stuff pytrin.
Your absolutely correct about the database classes. That is something that has been in the back of my mind.
As for the globals I'm not sure why I chose those over get_instance(). I'm pretty sure there was a reason (because I advocate against using them), but yet I did anyway. I should have commented that.
The framework isn't something I set out to design. It's kind of evolved over the years into what it is. Bits and pieces of different code that have somehow all globed together into a cohesive unit. As you can see it's incredibly fast, which is something that really stands out in IE. The pages aren't being cached either, because it sends headers to disable that.
I've tested the memory usage which is very low as well. That was something I was going to integrate into the framework but I discovered that the memory usage functions aren't universally available.
There is some older code in there that is due for refactoring. There's quite a bit of stuff I want to add to the framework but time is limited.
A major feature that isn't included in the version I released is a panel which allows you to create validation code from a database schema. That can turn 2 hours worth of work into 15 minutes as you only have to make minor changes to the validation code once you create it.
Anyway, thank you for your comments, I appreciate them.
Your absolutely correct about the database classes. That is something that has been in the back of my mind.
As for the globals I'm not sure why I chose those over get_instance(). I'm pretty sure there was a reason (because I advocate against using them), but yet I did anyway. I should have commented that.
The framework isn't something I set out to design. It's kind of evolved over the years into what it is. Bits and pieces of different code that have somehow all globed together into a cohesive unit. As you can see it's incredibly fast, which is something that really stands out in IE. The pages aren't being cached either, because it sends headers to disable that.
I've tested the memory usage which is very low as well. That was something I was going to integrate into the framework but I discovered that the memory usage functions aren't universally available.
There is some older code in there that is due for refactoring. There's quite a bit of stuff I want to add to the framework but time is limited.
A major feature that isn't included in the version I released is a panel which allows you to create validation code from a database schema. That can turn 2 hours worth of work into 15 minutes as you only have to make minor changes to the validation code once you create it.
Anyway, thank you for your comments, I appreciate them.
Re: Here guys - adeframework.com
"Accelerated Development Environment Framework Framework" is a weird name. Assuming the page <title> tag is correct. 
Re: Here guys - adeframework.com
Yeah I'll fix that.
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Here guys - adeframework.com
I don't mind looking at frameworks, regardless of license. It's not like I'm going to use your framework, basically for the same reasons you don't use others. But if I can steal an idea, it's worth my time. 
Questions/Suggestions:
1) Why do you have the cron folder inside your your framework? Doesn't linux already provide a cron folder? Or is that where you store maintenance scripts? if the latter, I would rename that to make it more clear.
2) What the heck is $_KERNEL about? I see 2 problems with using that. One, $_ I think is reserved for built in PHP super globals so I would stay away (personally). Two, Kernel is a bit of a misnomer. I see kernel and I think it's somehow wrapping actual Kernel functions in PHP.
3) What or where is your boot strap? includes/core/init.php is your bootstrap? I see a kernel::get_instance() which I assume is your interpretation of a front controller? Seem's it's actually more than a front controller though, as the object returned also initializes your templates, etc??? index.php also calls a rendering function on the kernel object.
Personally I think your delegating to much work to one object. Seems like you have the KERNEL object acting as a master class which handles requests (front controller) views (view manager of sorts) template initializastion, etc. You have something of a super facade going on.
includes/core/fuctions.php
A lot of generic functions in there get, post, etc. I'd suggest switching over to a request object to keep things more OOP.
You also have some functions in there which generate FORM code. Personally I would store those functions in separate well named files for clarity. Anything that generates HTML in my own framework is done in a template, always. I think I have encountered one situation where I needed to actually output HTML in code that wasn't template based.
That file also has functions which would be best in a response object, but your redirect() has a dependency on KERNEL so you might have some serious refactoring/redesigning to do if you wanted a more pure OOP approach.
I like your pagination class, at least in how you avoid any HTML inside the actual class and essentially just have the class initialize some variables which can then be used by a template to generate the actual pager HTML. Although it seems to have a concrete dependency on DB object which is dependent on KERNEL.
The classes buffer, compiler...are they for templates?
dbconnection class is redundant. Why not use PDO and save yourself a tonne of time and increase efficiency?
I notice you have the following in every class:
two concerns:
1) You don't explicitly create the member variable and set it's access control modifier.
2) You do this in each and every class.
Although it became pretty clear, pretty quick that the $this->kernel was an dynamic/expando property I still prefer to see the access control modifier for explicitness and clarity. I'm not sure if the property can be accessed publically in these cases, but I rarely (if ever) allow properties to be publically accessible -- private or protected at best.
My biggest conern with your classes in the amount of coupling and dependencies. Although they are simple classes on their own, things get wildly complex once you start trying to peice togather how the work to form a complete system. The coupling would make it fragile to change and likely require a re-write in order to change anything drastically.
Your file structure is a little, well, typical. I hate redundancy.
I notice you have:
I can't stand when projects do that, just a personal beef I have. In my applications I strive for absolute clarity and simplicity. If I'm looking for a class I know I have only one directory to look under, etc.
Most of what I complain about isn't really important to the original developer, but for someone just coming onto a project, these are always my concerns.
I do like the fact that your directory tree isn't 25 levels deep, that is another pet peeve of mine.
One last beef. index.php it's so empty??? Personally I find that such a waste as that is the first place I start when I enter a new project. I find it irritating when it starts calling includes and a single function. Why not keep the include expanded inside index.php -- unless there is an overwhelming reason to keep it as a external include???
I'm still quite bothered by your KERNEL object. After reading your docs breifly, it seems now that it's almost acting as a registry, when you say:
What are event triggers all about? I didn't bother trying to figure those out. What purpose do they serve? Are they hooks to allow core customiztion without tinkering with core code? Did you borrow that idea from CodeIgnitor?
Off Topic: While I agree arborint sometimes sounds like a grumpy old man.
His devils advocate approach to answering questions is usually very helpful. I hate to say it (just teasing) but I don't think there is a single person on these boards who has taught me more about my own best practices than him. If not directly, indirectly by at least making me second guess myself and try something different and sometimes that results in something better.
I do agree with the sentiment, that you want people to look at your code probono but have not made it publically usable. On the flip side, I don't really think in your case (astions) a license is a big deal. It's not like anyone is going to run off and use your code, anymore than arborints framework, my framework, or anyone else's on here.
If anyone made a PHP framework private in that regard and attempted to sell it, I'd laugh as all frameworks have their sucky parts and good parts.
Now how about looking over my application and offering some opinions?
I appreciate your sharing your code with us, I learn something new each time I look at code.
Cheers
Questions/Suggestions:
1) Why do you have the cron folder inside your your framework? Doesn't linux already provide a cron folder? Or is that where you store maintenance scripts? if the latter, I would rename that to make it more clear.
2) What the heck is $_KERNEL about? I see 2 problems with using that. One, $_ I think is reserved for built in PHP super globals so I would stay away (personally). Two, Kernel is a bit of a misnomer. I see kernel and I think it's somehow wrapping actual Kernel functions in PHP.
3) What or where is your boot strap? includes/core/init.php is your bootstrap? I see a kernel::get_instance() which I assume is your interpretation of a front controller? Seem's it's actually more than a front controller though, as the object returned also initializes your templates, etc??? index.php also calls a rendering function on the kernel object.
Personally I think your delegating to much work to one object. Seems like you have the KERNEL object acting as a master class which handles requests (front controller) views (view manager of sorts) template initializastion, etc. You have something of a super facade going on.
includes/core/fuctions.php
A lot of generic functions in there get, post, etc. I'd suggest switching over to a request object to keep things more OOP.
You also have some functions in there which generate FORM code. Personally I would store those functions in separate well named files for clarity. Anything that generates HTML in my own framework is done in a template, always. I think I have encountered one situation where I needed to actually output HTML in code that wasn't template based.
That file also has functions which would be best in a response object, but your redirect() has a dependency on KERNEL so you might have some serious refactoring/redesigning to do if you wanted a more pure OOP approach.
I like your pagination class, at least in how you avoid any HTML inside the actual class and essentially just have the class initialize some variables which can then be used by a template to generate the actual pager HTML. Although it seems to have a concrete dependency on DB object which is dependent on KERNEL.
The classes buffer, compiler...are they for templates?
dbconnection class is redundant. Why not use PDO and save yourself a tonne of time and increase efficiency?
I notice you have the following in every class:
Code: Select all
$this->kernel = kernel::get_instance();1) You don't explicitly create the member variable and set it's access control modifier.
2) You do this in each and every class.
Although it became pretty clear, pretty quick that the $this->kernel was an dynamic/expando property I still prefer to see the access control modifier for explicitness and clarity. I'm not sure if the property can be accessed publically in these cases, but I rarely (if ever) allow properties to be publically accessible -- private or protected at best.
My biggest conern with your classes in the amount of coupling and dependencies. Although they are simple classes on their own, things get wildly complex once you start trying to peice togather how the work to form a complete system. The coupling would make it fragile to change and likely require a re-write in order to change anything drastically.
Your file structure is a little, well, typical. I hate redundancy.
I notice you have:
Code: Select all
includes/classes/{empty}
includes/core/classes/{files}Most of what I complain about isn't really important to the original developer, but for someone just coming onto a project, these are always my concerns.
I do like the fact that your directory tree isn't 25 levels deep, that is another pet peeve of mine.
One last beef. index.php it's so empty??? Personally I find that such a waste as that is the first place I start when I enter a new project. I find it irritating when it starts calling includes and a single function. Why not keep the include expanded inside index.php -- unless there is an overwhelming reason to keep it as a external include???
I'm still quite bothered by your KERNEL object. After reading your docs breifly, it seems now that it's almost acting as a registry, when you say:
Sounds like a registry.The kernel allows you to easily call any class or method from anywhere in the code base
Now I'm thinking service locator.It will autoload classes located in includes/classes and includes/core/classes.
What are event triggers all about? I didn't bother trying to figure those out. What purpose do they serve? Are they hooks to allow core customiztion without tinkering with core code? Did you borrow that idea from CodeIgnitor?
Off Topic: While I agree arborint sometimes sounds like a grumpy old man.
I do agree with the sentiment, that you want people to look at your code probono but have not made it publically usable. On the flip side, I don't really think in your case (astions) a license is a big deal. It's not like anyone is going to run off and use your code, anymore than arborints framework, my framework, or anyone else's on here.
If anyone made a PHP framework private in that regard and attempted to sell it, I'd laugh as all frameworks have their sucky parts and good parts.
Now how about looking over my application and offering some opinions?
I appreciate your sharing your code with us, I learn something new each time I look at code.
Cheers
Re: Here guys - adeframework.com
Glad to see you poke your head in here.
That saves me a lot of time, especially on pages that contain lots of post or get variables.
My issue with that was that I didn't want to sprinkle $kernel->events->run('event_name_here'); all over the place, but I haven't found a better way yet.
Yes, it's just a place to store cron scripts. What would you rename it to? cron_scripts?Hockey wrote:1) Why do you have the cron folder inside your your framework? Doesn't linux already provide a cron folder? Or is that where you store maintenance scripts? if the latter, I would rename that to make it more clear.
The kernel is basically a registry object that will also instantiate classes if they don't exist yet. It contains a method that checks it can locate a class, which needs to be hooked to a resource locator, that I haven't created yet.Hockey wrote: 2) What the heck is $_KERNEL about? I see 2 problems with using that. One, $_ I think is reserved for built in PHP super globals so I would stay away (personally). Two, Kernel is a bit of a misnomer. I see kernel and I think it's somehow wrapping actual Kernel functions in PHP.
The init file includes all the files required for the framework to run. kernel::get_instance(); is just initializing the $_KERNEL var for later usage. init.php is also setting the default template.Hockey wrote: 3) What or where is your boot strap? includes/core/init.php is your bootstrap? I see a kernel::get_instance() which I assume is your interpretation of a front controller? Seem's it's actually more than a front controller though, as the object returned also initializes your templates, etc??? index.php also calls a rendering function on the kernel object.
The $_KERNEL doesn't do anything, you can just call other methods and classes *through* it.Hockey wrote: Personally I think your delegating to much work to one object. Seems like you have the KERNEL object acting as a master class which handles requests (front controller) views (view manager of sorts) template initializastion, etc. You have something of a super facade going on.
Most of those functions have very little code in them and server very simple purposes. They are designed to save typing. Like this:Hockey wrote: includes/core/fuctions.php
A lot of generic functions in there get, post, etc. I'd suggest switching over to a request object to keep things more OOP.
Code: Select all
# instead of typing
echo (isset($_POST['var'])) ? $_POST['var'] : '';
# I can do this
echo post('var');
Again, these are to simply save time. I really can't justify the overhead of using a template for an input box. These work fine until you need to start doing things such as adding javascript. You can just pass classes as an argument. If an input field has an error flagged in the form_error class it will be assigned an error class.Hockey wrote: You also have some functions in there which generate FORM code. Personally I would store those functions in separate well named files for clarity. Anything that generates HTML in my own framework is done in a template, always. I think I have encountered one situation where I needed to actually output HTML in code that wasn't template based.
Code: Select all
echo input('var_name');
Again I don't see a need to put something as simple as a redirect in a class. It calls the kernel so that the system can shutdown correctly. The problem is that PHP is destructing some of the classes required by other classes before their __destruct method had a chance to execute. Having the kernel shut down these classes first solves the problem.Hockey wrote: That file also has functions which would be best in a response object, but your redirect() has a dependency on KERNEL so you might have some serious refactoring/redesigning to do if you wanted a more pure OOP approach.
Yeah it needs some work but it's a real time saver. There are two issues with it. 1 is that it has a small bug I believe and 2 I would like to add the option to have it automatically add a limit clause rather than seeking to rows in large results set. Seeking to rows or using limit both have their drawbacks, but at least then the programmer could decide. It depends on the kernel because it requires a database connection. You can pass it your own db connection if you wanted it to use a mysql slave server for example.Hockey wrote: I like your pagination class, at least in how you avoid any HTML inside the actual class and essentially just have the class initialize some variables which can then be used by a template to generate the actual pager HTML. Although it seems to have a concrete dependency on DB object which is dependent on KERNEL.
They build the webpage.Hockey wrote: The classes buffer, compiler...are they for templates?
I like my database class better than PDO.Hockey wrote: dbconnection class is redundant. Why not use PDO and save yourself a tonne of time and increase efficiency?
This is how the classes access other classes - through the kernel. It would not be a bad idea to set this as private in each class.Hockey wrote:1) You don't explicitly create the member variable and set it's access control modifier.Code: Select all
$this->kernel = kernel::get_instance();
2) You do this in each and every class.
I'm not fond of dependencies either, but this is a mute point. I think all frameworks are like this.Hockey wrote: My biggest conern with your classes in the amount of coupling and dependencies. The coupling would make it fragile to change and likely
require a re-write in order to change anything drastically.
There's nothing preventing them from being placed into a single file. It's purely to keep the framework classes separate from yours.Hockey wrote: I notice you have:
Code: Select all
includes/classes/{empty} includes/core/classes/{files}
That is all it needs. Did you expect to see 5 require_once();Hockey wrote: One last beef. index.php it's so empty???
I haven't borrowed anything. The trigger system is something I was going to add so you execute classes or classes and methods based on events. For example you could add a page_not_found event that would execute a class which would then do something based on the requested resource.Hockey wrote: What are event triggers all about? I didn't bother trying to figure those out. What purpose do they serve? Are they hooks to allow core customiztion without tinkering with core code? Did you borrow that idea from CodeIgnitor?
My issue with that was that I didn't want to sprinkle $kernel->events->run('event_name_here'); all over the place, but I haven't found a better way yet.
Sure where's it at?Hockey wrote: Now how about looking over my application and offering some opinions?![]()
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Here guys - adeframework.com
probably just 'scripts' personally.Yes, it's just a place to store cron scripts. What would you rename it to? cron_scripts?
I figured that out eventually...see where I'm going with this? I would refactor that KERNEL into a few smaller objects. A registry, service locator perhaps.The kernel is basically a registry object that will also instantiate classes if they don't exist yet. It contains a method that checks it can locate a class, which needs to be hooked to a resource locator, that I haven't created yet.
I figured that out too eventually...but I'm confused as to the purpose? Why not just include the class file, instantiate the object and call it's members. It would be more efficient.The $_KERNEL doesn't do anything, you can just call other methods and classes *through* it.
I suppose if your KERNEL is a service locator and automates the include and instantiation of an object and then doubles as a registry/object stash -- it becomes more useful.
When you have the KERNEL instantiate objects though, how would you ever perform any kind of constructor injection? I dislike setter's -- I prefer to initialize an object with it's ctor(). I think there was a disscussion around here a while back about intelligent DI libraries which as I understand, acted like service locators, initializing the objects at construciton time via a config file.
I didn't either, until CC contributed to a thread I started. His reason was purely OOP but after implementing a response object I have since discovered many other really handy situations in which it has helped greatly.Again I don't see a need to put something as simple as a redirect in a class
You need to build a full fledge application I think to truely appreciate all these little objects working as a system.
I'm pretty sure PHP and most other languages implement GC as a LIFO stack. That is, object A is created first, object B is created second, object B is destroyed first, object A is destroyed last.The problem is that PHP is destructing some of the classes required by other classes before their __destruct method had a chance to execute. Having the kernel shut down these classes first solves the problem.
I think if your experiencings these problems, thats a design issue being introduced because of the coupling you have between classes.
While I find PDO a serious PITA -- I guess it depends on what your after. If it ain't broke, don't fix it.I like my database class better than PDO.
Yes they do, I agree, but some dependencies are better than others, either for the sake of clarity or simplicity. Other dependencies are abstract (IoC/DI) and others are concrete, such as having each class call upon your KERNEL class. Sorry I keep CAPS the KERNEL it's just force of habit after looking at your code.I'm not fond of dependencies either, but this is a mute point. I think all frameworks are like this
Haha...no thats no better. I just hate finding the index.php only to find a single line and now I have to follow through countless functions or includes to learn what or how the system pulls it self up from it's boot straps.That is all it needs. Did you expect to see 5 require_once();
I'm pretty sure it's CodeIgnitor which does that as well. Your framework is very similar, it too uses something like your kernel object to access all it's objects. I have ot be honest, I don't like that choice, but it obviously works for some people, CI is pretty popular.I haven't borrowed anything. The trigger system is something I was going to add so you execute classes or classes and methods based on events. For example you could add a page_not_found event that would execute a class which would then do something based on the requested resource.
Re: Here guys - adeframework.com
So I guess what I'm getting here is that a lot of you aren't fond of the $_KERNEL? Would it be better to call it something else?
This is an awesome framework. Creating a site with it using MVC would be trivially easy, the template system is easy to use and it's got a lot of other things going for it.
I'm surprised you guys aren't jumping on it. I'd worry more about what it does for the programmer than the code base. The core code base can always be changed/refactored, but it works virtually flawlessly as it is right now.
Is anyone interested in adding features/improving the code base? If so I'll open source it and throw it in google svn.
This is an awesome framework. Creating a site with it using MVC would be trivially easy, the template system is easy to use and it's got a lot of other things going for it.
I'm surprised you guys aren't jumping on it. I'd worry more about what it does for the programmer than the code base. The core code base can always be changed/refactored, but it works virtually flawlessly as it is right now.
Is anyone interested in adding features/improving the code base? If so I'll open source it and throw it in google svn.