RE: [PHP] Insult my code!
Hi Paul, As I agree some of your thoughts, I want to add my opinion also. Yes the code should work. That is why we earn Money. If it doesnt work, then we are on fire. But things like OOP or MVC weren't invented for a better running code. They are invented so the codes will going to be much more clean, readable, reusable, maintainable. Running codes is not enough. Eric asked about how his MVC structure looks. And we are trying to help what we know about MVC. He didn't asked if the code is fine for running. So giving an answer The real key is, does it work, and can it be maintained is not enough and not really helpful to him on MVC concept. If you need just a running and maintainable project, you don't need to use MVC (MVC is not all about that). We are not criticizing his code(the code is really fine(except injection problem :-) ) and very readable) Eric, As Martin said, All the business logic should be in Model. Controller should not tell a model Save it to this database, select it from this table, use this db_adapter etc. A controller is like a maestro of the system. It askes the model to play piano loud. But it wont say which key of piano, the model should touch. I suggest you to read this online book about Zend Framework and MVC. Its really really very helpful to understand the concept. Also example codes are very clean and good. http://www.survivethedeepend.com/zendframeworkbook/en/1.0 Take Care, Mert (sorry for my english) -Original Message- From: Paul M Foster [mailto:pa...@quillandmouse.com] Sent: Wednesday, October 07, 2009 7:54 PM To: php-general@lists.php.net Subject: Re: [PHP] Insult my code! On Wed, Oct 07, 2009 at 05:34:35PM +1100, Eric Bauman wrote: Hi there, I'm in the process of trying to wrap my head around MVC, and as part of that, I'm attempting to implement a super-tiny MVC framework. I've created some mockups of how the framework might be used based around a very simple 'bank', but I'm trying to get some feedback before I go and implement it, to make sure I'm actually on the right track. Any thoughts would be much appreciated! Model - http://www.pastebin.cz/23595 Controller - http://www.pastebin.cz/23597 View - http://www.pastebin.cz/23598 Template - http://www.pastebin.cz/23599 Your code (what there is of it) is fine. Beware of people who criticize your code on purely academic criteria. There are a lot of differing opinions about MVC, much of it driven by people making academic points versus people who actually code for a living. Even some people who actually code for a living fall under the spell of academic rules about this or that. The real key is, does it work, and can it be maintained. If so, don't worry about people who argue esoteric points about what should or shouldn't be in models and controllers, etc. Paul -- Paul M. Foster -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php Bu mesaj ve ekleri, mesajda g?nderildi?i belirtilen ki?i/ki?ilere ?zeldir ve gizlidir. Size yanl??l?kla ula?m??sa l?tfen g?nderen kisiyi bilgilendiriniz ve mesaj? sisteminizden siliniz. Mesaj ve eklerinin i?eri?i ile ilgili olarak ?irketimizin herhangi bir hukuki sorumlulu?u bulunmamaktad?r. ?irketimiz mesaj?n ve bilgilerinin size de?i?ikli?e u?rayarak veya ge? ula?mas?ndan, b?t?nl???n?n ve gizlili?inin korunamamas?ndan, vir?s i?ermesinden ve bilgisayar sisteminize verebilece?i herhangi bir zarardan sorumlu tutulamaz. This message and attachments are confidential and intended for the individual(s) stated in this message. If you received this message in error, please immediately notify the sender and delete it from your system. Our company has no legal responsibility for the contents of the message and its attachments. Our company shall have no liability for any changes or late receiving, loss of integrity and confidentiality, viruses and any damages caused in anyway to your computer system. -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
On 8/10/2009 1:20 AM, Andrea Giammarchi wrote: So far I stopped at the first line, the constructor, where I can spot with what I can read SQL injections everywhere I hope here is a proper validation there, 'cause as is, sounds truly dangerous, since you are not using bindParams or other PDO related techniques to avoid input problems. About the rest I kinda agree with the proper model controller, rather than just a reader. Regards To: php-general@lists.php.net Date: Wed, 7 Oct 2009 17:34:35 +1100 From: baum...@livejournal.dk Subject: [PHP] Insult my code! Hi there, I'm in the process of trying to wrap my head around MVC, and as part of that, I'm attempting to implement a super-tiny MVC framework. I've created some mockups of how the framework might be used based around a very simple 'bank', but I'm trying to get some feedback before I go and implement it, to make sure I'm actually on the right track. Any thoughts would be much appreciated! Model - http://www.pastebin.cz/23595 Controller - http://www.pastebin.cz/23597 View - http://www.pastebin.cz/23598 Template - http://www.pastebin.cz/23599 -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php _ Windows Live: Friends get your Flickr, Yelp, and Digg updates when they e-mail you. http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_3:092010 The linked code was supposed to be more of a mockup than anything, with the functions a bit of filler to try and show what I'm trying to do. With regard to the SQL injection, I try not to make the problems with my code quite so blatant. :-) -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
On 8/10/2009 5:18 PM, Mert Oztekin wrote: Hi Paul, As I agree some of your thoughts, I want to add my opinion also. Yes the code should work. That is why we earn Money. If it doesnt work, then we are on fire. But things like OOP or MVC weren't invented for a better running code. They are invented so the codes will going to be much more clean, readable, reusable, maintainable. Running codes is not enough. Eric asked about how his MVC structure looks. And we are trying to help what we know about MVC. He didn't asked if the code is fine for running. So giving an answer The real key is, does it work, and can it be maintained is not enough and not really helpful to him on MVC concept. If you need just a running and maintainable project, you don't need to use MVC (MVC is not all about that). We are not criticizing his code(the code is really fine(except injection problem :-) ) and very readable) Eric, As Martin said, All the business logic should be in Model. Controller should not tell a model Save it to this database, select it from this table, use this db_adapter etc. A controller is like a maestro of the system. It askes the model to play piano loud. But it wont say which key of piano, the model should touch. I suggest you to read this online book about Zend Framework and MVC. Its really really very helpful to understand the concept. Also example codes are very clean and good. http://www.survivethedeepend.com/zendframeworkbook/en/1.0 Take Care, Mert (sorry for my english) Thanks for the link, it looks like an interesting read. Hopefully it will help me understand MVC better and hence allow me to improve my code design. -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP] Insult my code!
Seems ok. Just a thought: Your model seems to be coded just for retreiving data. IMO you should code it for all possible actions(insert,update,delete,select). And also it should run without any database calls(you may create a new bank user in a page and use it than throw it away, so you wont have to need a database to save it). Your model may be more useable when it can holds the data itself(without retreiving it from db first), after that a save() method may inserts/updates it to db. -Original Message- From: Eric Bauman [mailto:baum...@livejournal.dk] Sent: Wednesday, October 07, 2009 9:35 AM To: php-general@lists.php.net Subject: [PHP] Insult my code! Hi there, I'm in the process of trying to wrap my head around MVC, and as part of that, I'm attempting to implement a super-tiny MVC framework. I've created some mockups of how the framework might be used based around a very simple 'bank', but I'm trying to get some feedback before I go and implement it, to make sure I'm actually on the right track. Any thoughts would be much appreciated! Model - http://www.pastebin.cz/23595 Controller - http://www.pastebin.cz/23597 View - http://www.pastebin.cz/23598 Template - http://www.pastebin.cz/23599 -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php Bu mesaj ve ekleri, mesajda gönderildiği belirtilen kişi/kişilere özeldir ve gizlidir. Size yanlışlıkla ulaşmışsa lütfen gönderen kisiyi bilgilendiriniz ve mesajı sisteminizden siliniz. Mesaj ve eklerinin içeriği ile ilgili olarak şirketimizin herhangi bir hukuki sorumluluğu bulunmamaktadır. Şirketimiz mesajın ve bilgilerinin size değişikliğe uğrayarak veya geç ulaşmasından, bütünlüğünün ve gizliliğinin korunamamasından, virüs içermesinden ve bilgisayar sisteminize verebileceği herhangi bir zarardan sorumlu tutulamaz. This message and attachments are confidential and intended for the individual(s) stated in this message. If you received this message in error, please immediately notify the sender and delete it from your system. Our company has no legal responsibility for the contents of the message and its attachments. Our company shall have no liability for any changes or late receiving, loss of integrity and confidentiality, viruses and any damages caused in anyway to your computer system.
Re: [PHP] Insult my code!
2009/10/7 Eric Bauman baum...@livejournal.dk: Any thoughts would be much appreciated! One observation. Model isn't a synonym for Database Table - models can be anything that encapsulates business logic. Requiring all your models to inherit from Model is probably a bad idea. -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
On 7/10/2009 7:36 PM, Mert Oztekin wrote: Seems ok. Just a thought: Your model seems to be coded just for retreiving data. IMO you should code it for all possible actions(insert,update,delete,select). And also it should run without any database calls(you may create a new bank user in a page and use it than throw it away, so you wont have to need a database to save it). Your model may be more useable when it can holds the data itself(without retreiving it from db first), after that a save() method may inserts/updates it to db. -Original Message- From: Eric Bauman [mailto:baum...@livejournal.dk] Sent: Wednesday, October 07, 2009 9:35 AM To: php-general@lists.php.net Subject: [PHP] Insult my code! Hi there, I'm in the process of trying to wrap my head around MVC, and as part of that, I'm attempting to implement a super-tiny MVC framework. I've created some mockups of how the framework might be used based around a very simple 'bank', but I'm trying to get some feedback before I go and implement it, to make sure I'm actually on the right track. Any thoughts would be much appreciated! Model - http://www.pastebin.cz/23595 Controller - http://www.pastebin.cz/23597 View - http://www.pastebin.cz/23598 Template - http://www.pastebin.cz/23599 -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php Bu mesaj ve ekleri, mesajda gönderildiği belirtilen kişi/kişilere özeldir ve gizlidir. Size yanlışlıkla ulaşmışsa lütfen gönderen kisiyi bilgilendiriniz ve mesajı sisteminizden siliniz. Mesaj ve eklerinin içeriği ile ilgili olarak şirketimizin herhangi bir hukuki sorumluluğu bulunmamaktadır. Şirketimiz mesajın ve bilgilerinin size değişikliğe uğrayarak veya geç ulaşmasından, bütünlüğünün ve gizliliğinin korunamamasından, virüs içermesinden ve bilgisayar sisteminize verebileceği herhangi bir zarardan sorumlu tutulamaz. This message and attachments are confidential and intended for the individual(s) stated in this message. If you received this message in error, please immediately notify the sender and delete it from your system. Our company has no legal responsibility for the contents of the message and its attachments. Our company shall have no liability for any changes or late receiving, loss of integrity and confidentiality, viruses and any damages caused in anyway to your computer system. Thanks for the response! You suggested that the model should run without any database calls. Do you mean that they should not exist in a model (ie. a controller should instantiate /and/ populate a model), or rather that it should be optional (ie. each method only updates the state of the object, and load() save() must be explicitly called to interact with the DB)? Or am I way off track. Cheers, Eric -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
On 7/10/2009 7:25 PM, David Otton wrote: 2009/10/7 Eric Baumanbaum...@livejournal.dk: Any thoughts would be much appreciated! One observation. Model isn't a synonym for Database Table - models can be anything that encapsulates business logic. Requiring all your models to inherit from Model is probably a bad idea. Thank-you for responding. Out of curiosity, why is this a bad idea? Is it also bad for views controllers? Cheers, Eric -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
On Wed, Oct 7, 2009 at 8:06 AM, Eric Bauman baum...@livejournal.dk wrote: On 7/10/2009 7:25 PM, David Otton wrote: 2009/10/7 Eric Baumanbaum...@livejournal.dk: Any thoughts would be much appreciated! One observation. Model isn't a synonym for Database Table - models can be anything that encapsulates business logic. Requiring all your models to inherit from Model is probably a bad idea. Thank-you for responding. Out of curiosity, why is this a bad idea? Is it also bad for views controllers? Cheers, Eric -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php It's about responsibilities. A Model should encapsulate business logic, and business logic and persistence has nothing in common. Maybe you can code a loader/persister object and let your models rely on it to handle the instances? This way you can decouple you business logic from the data persistence, allowing you to change one of them without introducing bugs into the other. Suppose you need to change your Database engine = change the loader/persister. You need to add/remove/modify some new rules in your business logic = change the models Why don't you allow the view to use the model? This way you have... # the controller is responsible to instantiate the model(s) and view(s) # the model is responsible for the business logic # the view is responsible for the presentation -- Martin Scotta
RE: [PHP] Insult my code!
So far I stopped at the first line, the constructor, where I can spot with what I can read SQL injections everywhere I hope here is a proper validation there, 'cause as is, sounds truly dangerous, since you are not using bindParams or other PDO related techniques to avoid input problems. About the rest I kinda agree with the proper model controller, rather than just a reader. Regards To: php-general@lists.php.net Date: Wed, 7 Oct 2009 17:34:35 +1100 From: baum...@livejournal.dk Subject: [PHP] Insult my code! Hi there, I'm in the process of trying to wrap my head around MVC, and as part of that, I'm attempting to implement a super-tiny MVC framework. I've created some mockups of how the framework might be used based around a very simple 'bank', but I'm trying to get some feedback before I go and implement it, to make sure I'm actually on the right track. Any thoughts would be much appreciated! Model - http://www.pastebin.cz/23595 Controller - http://www.pastebin.cz/23597 View - http://www.pastebin.cz/23598 Template - http://www.pastebin.cz/23599 -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php _ Windows Live: Friends get your Flickr, Yelp, and Digg updates when they e-mail you. http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_3:092010
Re: [PHP] Insult my code!
On Wed, Oct 07, 2009 at 05:34:35PM +1100, Eric Bauman wrote: Hi there, I'm in the process of trying to wrap my head around MVC, and as part of that, I'm attempting to implement a super-tiny MVC framework. I've created some mockups of how the framework might be used based around a very simple 'bank', but I'm trying to get some feedback before I go and implement it, to make sure I'm actually on the right track. Any thoughts would be much appreciated! Model - http://www.pastebin.cz/23595 Controller - http://www.pastebin.cz/23597 View - http://www.pastebin.cz/23598 Template - http://www.pastebin.cz/23599 Your code (what there is of it) is fine. Beware of people who criticize your code on purely academic criteria. There are a lot of differing opinions about MVC, much of it driven by people making academic points versus people who actually code for a living. Even some people who actually code for a living fall under the spell of academic rules about this or that. The real key is, does it work, and can it be maintained. If so, don't worry about people who argue esoteric points about what should or shouldn't be in models and controllers, etc. Paul -- Paul M. Foster -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
2009/10/7 Eric Bauman baum...@livejournal.dk: On 7/10/2009 7:25 PM, David Otton wrote: 2009/10/7 Eric Baumanbaum...@livejournal.dk: Any thoughts would be much appreciated! One observation. Model isn't a synonym for Database Table - models can be anything that encapsulates business logic. Requiring all your models to inherit from Model is probably a bad idea. Thank-you for responding. Out of curiosity, why is this a bad idea? Is it also bad for views controllers? Well, the way MVC has traditionally been approached is that the VC bit is a thin interface skin, concerned with I/O, while the M is the bulk of the program - the bit that does the heavy lifting. (You'll often hear this called Fat Model, Skinny Controller). So you could have a lot of models, that do a lot of disparate stuff - not just database tables. To give you an example, imagine an application that texts people when a new book by their favourite author is coming out. It's going to have models to represent database tables, the Amazon API and an SMS API. If all your controllers assume that all your models inherit from Model, then your code is monolithic and none of it can be reused. http://c2.com/cgi/wiki?InheritanceBreaksEncapsulation Of course, I'm talking about a general-purpose framework here - if all you're ever going to do is CRUD operations on DB tables, then by all means make assumptions about your models. You end up with all your classes tightly coupled, but they're still perfectly fit for purpose. -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
On Wed, Oct 07, 2009 at 09:09:29PM +0100, David Otton wrote: 2009/10/7 Eric Bauman baum...@livejournal.dk: On 7/10/2009 7:25 PM, David Otton wrote: 2009/10/7 Eric Baumanbaum...@livejournal.dk: Any thoughts would be much appreciated! One observation. Model isn't a synonym for Database Table - models can be anything that encapsulates business logic. Requiring all your models to inherit from Model is probably a bad idea. Thank-you for responding. Out of curiosity, why is this a bad idea? Is it also bad for views controllers? Well, the way MVC has traditionally been approached is that the VC bit is a thin interface skin, concerned with I/O, while the M is the bulk of the program - the bit that does the heavy lifting. (You'll often hear this called Fat Model, Skinny Controller). So you could have a lot of models, that do a lot of disparate stuff - not just database tables. To give you an example, imagine an application that texts people when a new book by their favourite author is coming out. It's going to have models to represent database tables, the Amazon API and an SMS API. If all your controllers assume that all your models inherit from Model, then your code is monolithic and none of it can be reused. http://c2.com/cgi/wiki?InheritanceBreaksEncapsulation I think this is a bit extreme. It really depends on what's in your parent model class. It could be something really simple, but something you don't want to have to rewrite in every model you code. Thinking that a model must stand on its own without inheriting from a parent model class because of some ideal notion of encapsulation is silly. Actually, in all deference to the eggheads of OOP, I believe there are really only four reasons to use classes: 1. It keeps me from having to rewrite the same code over and over (inheritance). 2. It keeps the namespace cleaner, since methods are unknown outside the context of their containing class. 3. It allows me to change details of function implementation without breaking things. This is a weak claim, though, because I can do the same thing with straight functions; just change the guts of the function, while still providing the same inputs and outputs. 4. It keeps the stack cleaner. I don't have to pass the same crap to every related function. I can keep the data inside my class and have all the methods share it. The notion that OOP was going to save the world billions of hours of programming time because of re-use is bunk. It hasn't, doesn't and won't. /rant Paul -- Paul M. Foster -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
2009/10/7 Paul M Foster pa...@quillandmouse.com: I think this is a bit extreme. It really depends on what's in your parent model class. It could be something really simple, but something you don't want to have to rewrite in every model you code. Thinking that Have you got an example of something that is needed by every model that interacts with a general-purpose framework? 1. It keeps me from having to rewrite the same code over and over (inheritance). Inheritance isn't the only mechanism for code-reuse, but it is the most tightly bound. In some situations it may be the appropriate solution, but it does force you into a taxonomy straightjacket. You just need to be aware of that, and pick the appropriate tool for the job. -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP] Insult my code!
On Wed, Oct 07, 2009 at 11:31:58PM +0100, David Otton wrote: 2009/10/7 Paul M Foster pa...@quillandmouse.com: I think this is a bit extreme. It really depends on what's in your parent model class. It could be something really simple, but something you don't want to have to rewrite in every model you code. Thinking that Have you got an example of something that is needed by every model that interacts with a general-purpose framework? Probably not a great example, but how about this: *Assuming* that the models need variables held in the config (which database, if any, for example), we put code in the constructor of the parent model which picks up these variables and stores references to them, for use in inherited models. The config could be a singleton class which holds a single instance of all the config variables. I *could* include a call like $this-config = get_config(); in each model's constructor. Or I could just do it once in the parent model. Of course, if this is all the parent model provides, then $this-config = get_config(); in each model would be roughly equivalent to parent::Model(); in each model. But if the parent provided more than that, then I would be writing less code to simply build it into the parent. And of course, if I ever wanted to *add* more to all the models, then having a parent model would allow me to do so without having to rejigger the code in each model. Paul -- Paul M. Foster -- PHP General Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
