PHP no nos

[waynew]

What bad practises/mistakes/misconceptions do you see time and time again - in regards to PHP?

 

inb4 or die(mysql_error())

[oni-kun]

What bad practises/mistakes/misconceptions do you see time and time again - in regards to PHP?

 

inb4 or die(mysql_error())

 

Statements without blocks, It's annoying to even look at (save 3 key presses!), such as:

if ($foo == $bar)
    DoThis();
echo 'Where does If end?!';

 

People assuming there are no errors being displayed, means there code is 'correct' and supposed to work.

I'm not going to do all that, because clearly there are no errors and it works

 

People using session_register();. Was the internet around in the 90's? Where are these PHP tutorials coming from?

 

People including files with a .inc extension without using .htaccess to parse it as PHP. It can be included, but what if you view site.com/includes/header.inc, You're screwed.

 

And last but not least,

or die("Uh oh, a query malfunctioned"); //Give out a useful error for debugging

 

Just rambling.. hehe.

[Lamez]

Code that repeats it self.

 

Like, say you have a table, and depending on what the user clicks, different info is loaded. I have see this done time, and time again. The table is copied and pasted all through out the script, when a function would save file size. In my opinion, that is just bad coding.

[Daniel0]

Code that repeats it self.

 

Like a quine?

[nrg_alpha]

• I would recommend always using error reporting (not in live production servers, but localhost - I suppose this can be configured in your installation as well - I'm not much of a server / let's configure everything kind of guy):
  

  error_reporting(E_ALL | E_STRICT);

  
   
  I have seen some people say it isn't necessary to use E_STRICT. Problem is, E_ALL won't catch the problem like this:
  

  class Foo{
      public function bar(){
  return 'I am a method!';
      }
  }
  
  echo Foo::bar();
  

  
   
  As of PHP 6, it is my understanding that E_STRICT will be rolled up into E_ALL (at least, that's what I read somewhere... can't recall where though).

---

• In classes, I tend to label private and protected properties with a leading underscore (for easier client coder readability):
  

  class Foo{
      public $name; // no leading underscore, as this property is public
      private $_age; // leading underscore tells me through the class code that this property is private
      ...
  }
  

---

• Mistakes I often run across include accidental assignments like:
  

  if ($a = $b) ...

---

• While the following isn't a mistake (nor perhaps a bad habit per say), it irks me to find someone do this:
  

  if($valuation == true)...

  as opposed to:
  

  if($valuation)...

  
  We know that a lone variable in that case is being evaluated to see if it equates to true or not.

---

• Bad formatting / indentation is an obvious given (too many examples to bother listing).

---

• While I confess to be guilty to this, sometimes, instead of declaring an array to start, like:
  

  $headhsot = array()

  
  I skip this and just start assigning values in a loop like:
  

  $headshot[] = $value

  
  Problem is, when I return to this code later on, I find myself wondering where this thing was declared.. and because it isn't, it confuses me.
  

---

• Another thing that bugs me is not giving variables meaningful names that make sense. Exmaple, did you know that $x is actually representative of a car budget? Me neither.. but $carBudget certainly does.
  

 

I'm sure I'll think of more...

 

 

Edit (Daniel0): Replaced faux horizontal rules with real ones.

[nrg_alpha]

lol @ quine.

[RichardRotterdam]

does it need an explanation?

if($something==$somethingelse){
for($a=0;$a<$max;$a++){
for($b=0;$b<$max;$b++){
if($code=="crap"){
}
}
}

[oni-kun]

$headhsot = array()

 

[nrg_alpha]

I'm not referring to variables in a loop acting as a counter.. I mean just creating a variable to represent something, but is poorly represented.

[nrg_alpha]

$headhsot = array()

 

lol Yes, I accumulate plenty of those off of opponents (virtually speaking of course.. so don't send the men in black after me, please. Kthanx).

[PugJr]

does it need an explanation?

if($something==$somethingelse){
for($a=0;$a<$max;$a++){
for($b=0;$b<$max;$b++){
if($code=="crap"){
}
}
}

 

 

 

 

Besides having wrong syntax (Lacking a "}" at the end) what exactly is the reason you do not like that or is wrong to do in programming?

[waynew]

<?php
mysql_connect("localhost","root","") or die(mysql_error());
mysql_select_db("myDB");

$pass = addslashes($_POST['password']);
$username = addslashes($_POST['username']);

$select = mysql_fetch_array("select * from user where username = '$username' and password = '$pass'");

if(mysql_num_rows($select) > 0){
header('Location: cp.php');
} else {
die('LOGIN INCORRECT PLEASE GO BACK');
}
?>

[oni-kun]

$select = mysql_fetch_array(mysql_query("select * from user where username = '$username' and password = '$pass'"));

 

? Double negative!

[Maq]

I would have to say one of my biggest pet peeves in developing is when people don't use any styling what so ever.  No indentation, no formatting, no consistency...  Makes me sick! 

[waynew]

• People assuming that POST values exist.
  
• The use of addslashes for escaping dangerous characters.
  
• People not using exit(); after their redirects.
  

I'll stop now.

[oni-kun]

I would have to say one of my biggest pet peeves in developing is when people don't use any styling what so ever.  No indentation, no formatting, no consistency...  Makes me sick! 

 

Tis why I put a link in my sig. 

[KevinM1]

does it need an explanation?

if($something==$somethingelse){
for($a=0;$a<$max;$a++){
for($b=0;$b<$max;$b++){
if($code=="crap"){
}
}
}

 

 

 

 

Besides having wrong syntax (Lacking a "}" at the end) what exactly is the reason you do not like that or is wrong to do in programming?

 

The complete lack of indentation doesn't strike you as being hideous?

[PugJr]

 

The complete lack of indentation doesn't strike you as being hideous?

 

I wasn't sure as it seemed like he might have been pointing out something other than a lack of indents. Maybe he was saying having a loop in a loop is bad. It just seems that if thats his point, he wouldn't have included the loops.

[nrg_alpha]

I think he was referring to my comment about bad variable names (like $x representing say a car budget). $x was probably a bad example, as he probably thought about using variables inside loops as counters. In hindsight, I should have used say $cb (for car budget) instead.

 

But, like they say, hindsight has 20/20 vision. 

[shlumph]

I've seen it time and time again. Especially on these forums. Printing out HTML line by line!

<?php
print '<table><tr><td>' . "\r\n";
print 'this is a line of html' . "\r\n";
print '<strong>this is another line</strong>' . "\r\n";
print '</td></tr>' . "\r\n";
print '</table>' . "\r\n";

foreach($stuff as $s)
{
     print $s->stuff . 'asdfasdf' . "\r\n";
     print 'asdfasasdfasdfdf' . $s->moreStuff . "\r\n";
     print 'asdfasa'. $s->evenMoreStuff .'sdfasdfdf' . "\r\n";
}

 

I can't stand it!!!! Do this instead:

<table>
<tr>
	<td>
		This is a line of html
		<strong>this is another line</strong>	
	</td>
</tr>
</table>
<?php foreach($stuff as $s): ?>
<?= $s->stuff ?> asdfasdf
asdfasasdfasdfdf <?= $s->moreStuff ?>
asdfasa <?= $s->evenMoreStuff ?> sdfasdfdf
<?php endforeach; ?>

 

[.josh]

I've seen it time and time again. Especially on these forums. Printing out HTML line by line!

<?php
print '<table><tr><td>' . "\r\n";
print 'this is a line of html' . "\r\n";
print '<strong>this is another line</strong>' . "\r\n";
print '</td></tr>' . "\r\n";
print '</table>' . "\r\n";

foreach($stuff as $s)
{
     print $s->stuff . 'asdfasdf' . "\r\n";
     print 'asdfasasdfasdfdf' . $s->moreStuff . "\r\n";
     print 'asdfasa'. $s->evenMoreStuff .'sdfasdfdf' . "\r\n";
}

 

I can't stand it!!!! Do this instead:

<table>
<tr>
	<td>
		This is a line of html
		<strong>this is another line</strong>	
	</td>
</tr>
</table>
<?php foreach($stuff as $s): ?>
<?= $s->stuff ?> asdfasdf
asdfasasdfasdfdf <?= $s->moreStuff ?>
asdfasa <?= $s->evenMoreStuff ?> sdfasdfdf
<?php endforeach; ?>

 

 

okay, I can understand doing that for large amounts of html but seriously what you have there makes it less readable IMO....  Why not just use heredoc instead?

[Philip]

$select = mysql_fetch_array(mysql_query("select * from user where username = '$username' and password = '$pass'"));

? Double negative!

$select = mysql_fetch_array(mysql_query("select * from user where username = '$_POST[username]' and password = '$_POST[pass]'"));
echo $select[someKeyWithoutQuotesThatIsntAConstant];

[oni-kun]

$select = mysql_fetch_array(mysql_query("select * from user where username = '$_POST[username]' and password = '$_POST[pass]'"));
echo $select[someKeyWithoutQuotesThatIsntAConstant];

 

Or even worse,

for ($i=0;$i=20;$i+=$i); {
    $select['$i'];
}

[oni-kun]

for ($i=0;$i=20;$i+=$i); {
    $select['$i'];
}

 

Ok here's the fix.. <insert fix>

 

how do i put this in my code? i added it ot my own and it came up an error, any other ideas?

[waynew]

$select = mysql_fetch_array(mysql_query("select * from user where username = '$username' and password = '$pass'"));

? Double negative!

$select = mysql_fetch_array(mysql_query("select * from user where username = '$_POST[username]' and password = '$_POST[pass]'"));
echo $select[someKeyWithoutQuotesThatIsntAConstant];

 

You just made me rage.