Page 1 of 5
Bad practice!
Posted: Sat Mar 06, 2004 2:41 am
by patrikG
What would you consider to be bad practice when coding PHP?
Rant all you want against something you really, really sneer at, but at the end of your post, say how it should be done.
Personally, what I find very bad practice is mixing PHP with an HTML-page. Even worse, assigning each line of HTML to one PHP variable. Not only does it look ugly, it makes life so much more difficult in that logic and presentation are so entwined that you just see escaped quotes here there and everywhere, have to search for every little snippet of code that has anything to do with logic and then, once you've made the changes required, someone decides that it should actually look entirely different, but have the same, yet somewhat advanced, functionality.
Mixing PHP and HTML is like landfills. You dig a hole, pour concrete in and dump everything into it. Once it's full, you move on to the next.
Why not think about recycling. Think about re-using your code. Think how to abstract from what you have. There are so many excellent examples on the internet (and so many very bad ones) - go have a look at them, think about it the code first, think what you want to achieve, how to best structure it, then implement it. PHP can be so much more than a simplistic templating engine.
Posted: Sat Mar 06, 2004 7:16 am
by redhair
What template engine do you use?
I find myself using Yapter lately.
http://yapter.sf.net
Posted: Sat Mar 06, 2004 7:26 am
by patrikG
Heh, I wasn't talking about templates, but I am using my own template engine - if Manuel Lemos accepts it, it'll be on phpclasses.org soon.
Posted: Sat Mar 06, 2004 9:24 am
by redhair
You did say "PHP can be so much more than a simplistic templating engine.".
And the problem you sketch out, can best be solved using templates.
With "you" I meant "All of you reading this".
So what do "You" use?
I'm just curious about the result.
Posted: Sat Mar 06, 2004 10:09 am
by patrikG
At one level, I agree, redhair - the problem could easily be solved with a templating engine. But what I was getting at was that when you start programming, you should
a) have a very clear idea of what you want
b) think about how to make the code re-usable
c) use object oriented programming
d) at some point in the distant future start reading up on design patterns
Actually, onle a) and b) were in my post above, but why the kind of "bad practice" coding I mentioned above annoys me, is
a) I've done it
b) I've done it too many times
c) It cost me hours/days/weeks to modify things
d) seeing others repeat the same mistakes over and over and over again
e) at work I am the one who has to maintain and fix the code.
Re: Bad practice!
Posted: Sat Mar 06, 2004 11:39 am
by McGruff
patrikG wrote:Personally, what I find very bad practice is mixing PHP with an HTML-page.
That would be the first thing on my list.
Also, some basic refactoring techniques:
(1) wrap code in short functions which do just one thing; 6-7 lines is a good size
(2) anything which is repeated should (usually) be resolved into a single source
Posted: Sat Mar 06, 2004 11:52 am
by m3mn0n
My list:
1. Lack of script structure
2. Lack of spacing
3. Ambundance of HTML within PHP (or vice versa)
4. Unorganized application structure
5. Classes that do too much
6. Unnessesary use of functions
7. Too much comments
8. Not enough comments
To explain those points would require me writing a book, sorry.

Posted: Sat Mar 06, 2004 11:58 am
by McGruff
Sami wrote:
6. Unecessary use of functions
Not sure about this one: in fact I'd almost say you can't have too many.
I hardly ever have code that isn't wrapped up in a fn (or rather a class method) and you never regret refactoring big functions down into smaller parts.
Of course, I don't know exactly what you have in mind in mentioning this.
Posted: Sat Mar 06, 2004 12:35 pm
by timvw
If love scripts that make use of global variables and that start like this
<?
if ($is_logged_in) {
// send lotz of money to user
}
?>
I don't really like short tags either, but then again, this is for the style police.

Posted: Sat Mar 06, 2004 1:15 pm
by patrikG
I agree Sami & McGruff, also what I find "highly improvable", i.e. annoying, is when you read code and it's all in those one-character variables $a to $z. At best maybe even two characters.
Jeez, the machine understands the code anyway, but us humans have to maintain it. Make the code as readable as possible and if you return to a piece of code written 10 months ago, you'll find your way around much more easily.
Posted: Sat Mar 06, 2004 2:01 pm
by McGruff
Exactly: I'd say about two thirds of the effort goes into writing easily understandable & maintainable code - good names, clear design, etc.
Posted: Sat Mar 06, 2004 2:23 pm
by Bill H
1. Unnecessary use of functions
I agree with Sami that functions can be overused. Any function that performs a highly specialized routine and is only called one time in an entire routine (is so specialized that it can only be called one time) and is no more than a few lines of code merely serves to obscure what is going on. I see that done more often than you might think.
2. Over-use of object oriented programming.
Using classes purely for the sake of using classes does not help the routines. For small, simple routines (sites) it merely makes things more unwieldy and obscure.
I have discarded third-party scripts for both of the above reasons, because they were so obscured by the overuse of classes and functions that they were virtaully impossible to decipher and modify.
Remember the KISS principle. (Keep It Simple Stupid)
Posted: Sat Mar 06, 2004 3:33 pm
by McGruff
Bill H wrote:1. Unnecessary use of functions
I agree with Sami that functions can be overused. Any function that performs a highly specialized routine and is only called one time in an entire routine (is so specialized that it can only be called one time) and is no more than a few lines of code merely serves to obscure what is going on. I see that done more often than you might think.
I respectfully but strongly disagree with this. The purpose of functions is to isolate discrete tasks. It doesn't matter how many times each task is performed in a script: stick 'em in their own function. This creates chapters and titles in the code greatly increasing readability imo.
For example, management tasks suddenly become clearer for what they are:
Code: Select all
<?php
function selectAction($parameter)
{
if(isValid($parameter))
{
doThis();
} else {
doThat();
}
}
?>
Notice that the test has been factored into its own isValid function, as well as the actions for each case. If these were very simple, agreed there isn't much point but anything more than a few lines would trigger a rewrite imo.
You don't even have to understand any php to figure out selectAction. It's immediately clear it doesn't actually do anything itself: it decides what should be done. That's all you want to know about it. A script workflow decision has been made that much clearer and easier to track down.
I've seen (and written) a million fns where the whole caboodle gets lumped into one, indecipherable function with dozens of lines or more. The workflow decision gets obscured by the actions being performed and vice versa. You shouldn't have to take 10minutes to figure out what a fn does. If they are short, single-shot fns you don't have to.
Bill H wrote:2. Over-use of object oriented programming.
Using classes purely for the sake of using classes does not help the routines. For small, simple routines (sites) it merely makes things more unwieldy and obscure.
Personally I find fns very unwieldy for anything much more complicated than "hello world". I honestly wouldn't know where to start if I had to try to work with fns only.
There's good OOP and bad OOP. If procedural scripts can be written badly, then I guess that goes double for OOP. But, the good stuff is very good. It's not slow or unwieldy or verbose or any of the usual criticisms levelled at object oriented code. As with fns, ruthlessly refactor classes to do one thing only.
I have a forum with a reasonably big index page - about 20 or so separate boards. There are about 30 separate classes in the script (and I haven't finished refactoring or added a db layer yet) but the whole page takes about 3 or so tenths-of-a-second (on an ancient Duron 700).
Don't be afraid of splashing out on fns or classes - just make sure they're good ones

Posted: Sat Mar 06, 2004 3:40 pm
by McGruff
PS: hope you don't mind patrick but I thought this would be good for the theory & design board.
Posted: Sat Mar 06, 2004 4:44 pm
by Bill H
Code: Select all
<?php
function printhdr($Param)
{
echo "Whatever " . $Param . " rest of line<br><br>";
}
?>
This function was in a separate file, in a separate directory, and it was used once in the entire project. Sorry, McGruff, I cannot see that as being in any way useful. (It was one of quite a few similar cases in the project.)
2. Over-use of object oriented programming.
I probably should have added
3. Not using object oriented programming when needed.