XenoPhage Posted March 6, 2006 Share Posted March 6, 2006 Maybe I'm missing something here, but I see a lot of code moving through here that just makes my head ache. Trying to keep HTML, Javascript, AND PHP together in the same file is asking for trouble.. And I see a lot of craziness with SQL statements too..For instance, I see something like this :[code]<?php $test = 'Hello'; $test2 = 'World';?><html><head></head><body><h1><?php echo "$test" ?> <?php echo "$test2" ?></h2></body></html>[/code]UGH! Ugly! Why not use something like Smarty templates and make this much more readable?[code]<?php // Create a new smarty template and set up the globals for it include('Smarty.class.php'); $smarty = new Smarty; $smarty->compile_dir = "/usr2/smarty/templates_c"; $smarty->cache_dir = "/usr2/smarty/cache"; $smarty->config_dir = "/usr2/smarty/config"; $smarty->template_dir = dirname($_SERVER['PATH_TRANSLATED']) . '/templates'; $test = 'Hello'; $test2 = 'World'; $smarty->assign('test', $test); $smarty->assign('test2', $test2); $smarty->display('helloworld.tpl');?>[/code]And then in your template :[code]<html> <head> </head> <body> <h1>{$test} {$test2}</h1> </body></html>[/code]Yep, there's more initial code, BUT.. The big payoff here is being able to use the same templates for multiple pages, being able to easily change the template without impacting the main PHP code, and just plain READABILITY! In fact, the savvy coder will move the initial setup for smarty into a function and just include it at the beginning of the program. I spend most of my time here trying to differentiate the PHP from the HTML in most of these posts..And SQL... I see a lot of this :[code] $query = 'SELECT id, name, code FROM users WHERE id=' . $id_val . ' AND name=' . $name_val . ' ORDER BY code';[/code]Aside from being ugly, this can be relatively unsafe.. Add some safety in here. Besides sanitizing your variables, use sprintf and force the type :[code] $query = sprintf('SELECT id, name, code FROM users WHERE id=%d AND name="%s" ORDER BY code', $id_val, $name_val);[/code]There. The developer can quickly note that id is apparently a decimal value whereas name is obviously a string.Quite simply, using some strict coding guidelines, commenting, and keeping the various languages separate will help tremendously when debugging! Quote Link to comment https://forums.phpfreaks.com/topic/4263-making-life-easy/ Share on other sites More sharing options...
php_b34st Posted March 6, 2006 Share Posted March 6, 2006 Great post, I agrre with your suggestions. Quote Link to comment https://forums.phpfreaks.com/topic/4263-making-life-easy/#findComment-14817 Share on other sites More sharing options...
obsidian Posted March 6, 2006 Share Posted March 6, 2006 i agree with you [b]FOR THE MOST PART[/b]... let's go back to some of the examples you used:[code]<?php $test = 'Hello'; $test2 = 'World';?><html><head></head><body><h1><?php echo "$test" ?> <?php echo "$test2" ?></h2></body></html>[/code]while i agree that the obove is extremely sloppy code, i would NEVER suggest someone use the overhead of a template system for a simply output to the screen. in many cases on here, we deal with small snippets of code that WOULD NEVER be justified to use a template with because of the additional overhead the script would take to run. if you have many pages that are all going to use the same templating structure, i concur: people would be much better off using the template system. i would simply recommend adjusting the code above to something like this (after learning the difference between quote types, of course):[code]<?php$test = "Hello";$test2 = "World!";?><html><head></head><body><?= "<h1>$test $test2</h1>\n"; ?></body></html>[/code]i would also encourage people to use a valid doctype and valid markup in their real code... not like what is mentioned here.as for your SQL reference, i'll disagree slightly there: the code you initially posted does indeed contain some wasted space, but again, with a proper understanding of quotes, we can quickly clean up that statement to:[code]$query = "SELECT id, name, code FROM users WHERE id = '$id_val' AND name = '$name_val' ORDER BY code';[/code]i agree about sanitizing your variables to avoid SQL injection and other attacks, but hopefully, by the time you have your $_POST split into variables to put in a SQL statement like this, it's already been sanitized. i do, however agree 100% with your suggestion to use something such as sprintf() to format your SQL queries... not because it's easier to write, not by a long shot, but because it's MUCH easier to read. working in an environment where i have to trouble shoot other coders' work, it would be much easier if they had some sort of coding guidelines they had been following.when you're perusing code here on the forums, let's give people the benefit of the doubt... most users are going to have enough sense to document their code and use somewhat standard coding procedures (at least we hope they will), but this is a community where time is of the essence. there is NO WAY i'm going to spend the time to write up a function for someone in 15 minutes using all the guidelines i use in my own work when i can write the same basic thing up in 5 minutes using relatively "sloppy" code. the idea is to give people some [b]direction[/b], not always solve their problem for them. with that in mind, often times, the code you see written on here is an "off the handle" snippet intended for someone to [i]rewrite[/i] it in their own way away from here... gleaning what they can from the provided code.let me follow all that up by saying, in personal coding practices, i believe you've hit the nail on the head, but in a community such as this, it's really not practical to expect people to follow those guidelines whenever they're posting a snippet of code ;-) Quote Link to comment https://forums.phpfreaks.com/topic/4263-making-life-easy/#findComment-14872 Share on other sites More sharing options...
XenoPhage Posted March 7, 2006 Author Share Posted March 7, 2006 [!--quoteo(post=352319:date=Mar 6 2006, 06:59 PM:name=obsidian)--][div class=\'quotetop\']QUOTE(obsidian @ Mar 6 2006, 06:59 PM) [snapback]352319[/snapback][/div][div class=\'quotemain\'][!--quotec--]i agree with you [b]FOR THE MOST PART[/b]... [/quote]Uh oh.. :)[!--quoteo--][div class=\'quotetop\']QUOTE[/div][div class=\'quotemain\'][!--quotec--]while i agree that the obove is extremely sloppy code, i would NEVER suggest someone use the overhead of a template system for a simply output to the screen. in many cases on here, we deal with small snippets of code that WOULD NEVER be justified to use a template with because of the additional overhead the script would take to run. if you have many pages that are all going to use the same templating structure, i concur: people would be much better off using the template system. i would simply recommend adjusting the code above to something like this (after learning the difference between quote types, of course):[/quote]I was afraid someone would point that out.. Yes, I agree. If we're talking about small, quick snippets of code, then I agree. However, even if you don't use a templating system like Smarty, you can still break *most* HTML code out to a separate template file that is purely PHP. I personally don't like doing this, but it can make things neater.. But if we're talking about a small page to redirect, or display a quick message, then it's fine. Just as a point, Smarty is extremely fast and the extra overhead is almost negligible in my experience...[!--quoteo--][div class=\'quotetop\']QUOTE[/div][div class=\'quotemain\'][!--quotec--]i would also encourage people to use a valid doctype and valid markup in their real code... not like what is mentioned here.[/quote]Absolutely. And if you go that far, please check your HTML output with one of the many validators out there.[!--quoteo--][div class=\'quotetop\']QUOTE[/div][div class=\'quotemain\'][!--quotec--]i agree about sanitizing your variables to avoid SQL injection and other attacks, but hopefully, by the time you have your $_POST split into variables to put in a SQL statement like this, it's already been sanitized. i do, however agree 100% with your suggestion to use something such as sprintf() to format your SQL queries... not because it's easier to write, not by a long shot, but because it's MUCH easier to read. working in an environment where i have to trouble shoot other coders' work, it would be much easier if they had some sort of coding guidelines they had been following.[/quote]Hrm.. Well, I agree that sanitization is great, but by using sprintf() you gain a little extra security. If you use decimals or floats, and someone populates a variable with a string, it won't make it through into the query string. Yes, it shouldn't get that far. But, it happens.. Multiple layers of security are a good thing.. :)[!--quoteo--][div class=\'quotetop\']QUOTE[/div][div class=\'quotemain\'][!--quotec--]when you're perusing code here on the forums, let's give people the benefit of the doubt... most users are going to have enough sense to document their code and use somewhat standard coding procedures (at least we hope they will), but this is a community where time is of the essence. there is NO WAY i'm going to spend the time to write up a function for someone in 15 minutes using all the guidelines i use in my own work when i can write the same basic thing up in 5 minutes using relatively "sloppy" code. the idea is to give people some [b]direction[/b], not always solve their problem for them. with that in mind, often times, the code you see written on here is an "off the handle" snippet intended for someone to [i]rewrite[/i] it in their own way away from here... gleaning what they can from the provided code.[/quote]Yeah, I can agree with that.. I know the forum is intended for a quick answer, but it's still good to post besdt practices ideas... Some people need the extra reminder... :) Quote Link to comment https://forums.phpfreaks.com/topic/4263-making-life-easy/#findComment-14930 Share on other sites More sharing options...
wickning1 Posted March 7, 2006 Share Posted March 7, 2006 I'm all for having guidelines, but I personally dislike Smarty quite a bit. I find it very confusing and cumbersome. Your use of sprintf() strikes me as distasteful too. I built myself an abstraction layer to do that, that looks much nicer.I think elegant and readable code is just a matter of experience. Little by little you learn the basic issues and deal with them. Eventually you're prepared to set up some personal guidelines, or build a basic framework to work in (that's what I recommend).Everybody will have different solutions, even teams who've worked together for a long time and developed a system that works wonderfully for them turn out code that is complete nonsense to another team.You do definitely need to have a way to set apart HTML, client code (javascript), and server code (php). You also need security standards for protecting against injections, session theft, and controlling user permissions. Beyond that, it's hard to say which way is the best. I have a lot of ideas, I'm working on releasing them as a package, but even then, the code is only as elegant as the programmer. Quote Link to comment https://forums.phpfreaks.com/topic/4263-making-life-easy/#findComment-14945 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.