

Signs of Crappy PHP Software - lox
http://www.phpfreaks.com/blog/10-signs-of-crappy-php-software

======
chime
I seriously don't understand why people hate any use of globals? I've written
a 20k line MRP system that spans over 100 different PHP files. I have a global
"$look_ahead = 90; // days". Every once in a while, someone from production
planning comes up to me and asks if they can look ahead 45 - 120 days from now
on. I simply change that variable and all is good immediately. Every single
PHP file uses the global $look_ahead variable.

Sure, I could make a function look_ahead. Or I could pass the look_ahead value
everywhere. Or I could have a DB setting look_ahead. But why would I make it
any more complicated than I have to? Globals work very well when used as
application preferences that rarely change but could.

Also, $company_address is a global. We're moving to a new building. That's a
single variable I need to change. It'll change across every single
report/printout/auto-generated PDF. And $db is always a global in my code.
I've been coding for 10 years and not once have I had any issues because of
that.

edit: Why the downvotes? I thought the proper etiquette here was to post a
reply. It's not like I said something offensive.

~~~
_Lemon_
You're specifically using it for configuration/preferences though. You should
consider changing it to a constant instead as that's what they really are.

The problem is passing state around in global variables. Any code can (and
readily does) modify the state which is the problem.

~~~
chime
$db wouldn't work as a constant.

You're right that look_ahead/company_address would work as a
define("CONSTANT") better but honestly, I hate not putting a $ sign in front
of a value (constant or variable). I am more prone to missing a $ than
redefining a global. I don't remember ever having the problem of redefining a
global unintentionally. I can give 20 examples of when I missed a $ symbol and
PHP gave me an error/notice. If PHP constants had a symbol akin to $ (say #),
I would definitely use them. But I just feel dirty typing "echo LOOK_AHEAD;"
in one place and "echo $look_ahead_partial;" in another because I don't want
to get in the habit of putting $ in some places and not putting them in
others.

~~~
nir
On the other hand, when someone reads "echo LOOK_AHEAD" they immediately know
it's a constant and thus (a) global and (b)(more importantly) defined once and
never modified by the code.

With "echo $look_ahead" there's no indication of that, and in the future
another dev might add something that modifies $look_ahead's value and break
other parts of the code, making for sneaky bug (which incidentally is why non-
constant globals are so disliked ;))

------
alttab
I'm going to disagree with #9 that says using a core framework is bad. Now, if
the core framework isn't a pre-built web framework like CakePHP, Symphony,
there's a good chance there is some ... interesting things in there. But
honestly having a "core" at least tries to reuse code.

~~~
snorkel
Also disagreeing with #9. Plenty of popular frameworks have "core". core
provides a clear delineation between extensible parts of a framework and its
inner foundation. A well designed framework allows you to extend its
functionality and upgrade the core at any time without disruption. Frameworks
have core for the same reason objects have private members: interface
opaqueness provides change protection.

~~~
DanHulton
I think that if you're using a well-developed framework, you can safely ignore
#9. However, his advice is SPOT-FREAKING-ON for any home-grown stuff.

The last time I ran across a "core" directory, there was all kinds of
application-specific code in there, all rolled up in switches and stuff - if
you wanted to develop something using the "core", you had to go through about
fifty files and add your own cases for your site. It was awful.

------
petervandijck
Another sign (and that goes for any software): bad commenting and bad naming.
If the naming and commenting is good, you most likely have a programmer who
cares, which is half of the battle.

~~~
arnorhs
So true. In fact, I think this argument outweighs any or even all of the
author's arguments.

------
pacemkr
I doubt there are many who would argue that Drupal is crappy software. And
here's the shocker: it uses globals and most of it is procedural PHP! Let that
sink in... Signs alone do not prove guilt. =P

But -- just to be a hypocrite -- my number one sign of crappy software is
deeply nested if statements. The more you branch the more loose ends you have
to juggle up in the air as your solution is formulating.

Just to be productive, allow me to share a tool that has evaded me for many
years: assertions. No really!

    
    
      function GoodFunction
         assert(good_state)
         do_one_thing_very_well
    

Reads better, and leads to better design than:

    
    
      function GoodFunction?
        if( oh_right )
          do_one_thing_very_well_slightly_differently
        else
          do_one_thing_very_well
    

In the first the oh_right condition just doesn't fit in and you start thinking
about what caused this logical branch in the first place. In the second we
feel little hesitation to add an elseif if need be, further increasing
complexity.

To give credit where its due, "Coders at Work" has actually changed my coding
style. I highly recommend the book.

------
petervandijck
Another sign: check the mysql queries. If they don't properly escape
variables, danger.

~~~
jacksoncarter
if they escape variables: danger!

Escaping is not sufficient to prevent SQL Injection Attacks. You must used
Parameterized Queries.

<http://bobby-tables.com/>

~~~
chime
> Escaping is not sufficient to prevent SQL Injection Attacks.

Do you have an example or idea of how a SQL injection could occur despite
using [http://php.net/manual/en/function.mysql-real-escape-
string.p...](http://php.net/manual/en/function.mysql-real-escape-string.php) ?

------
neovive
Also, using data from $_GET, $_POST directly in Views.

------
wanderr
We are still suffering from the effects of a developer who employed #1
throughout the codebase. We have both Object and Collection, and bypassing his
classes results in reduuced memory usage by a factor of 10 and improved
performance by a factor of 100. Unforunately he also employed #7 so it's very
tricky to bypass that in some places without doing a major rewrite...

------
ojbyrne
"8. The software messes with the error level"

I did this one recently, because I needed to use some PEAR modules that
produced notices under E_STRICT. I somehow think all of these have similar
exceptions.

~~~
arnorhs
Yes, you often want to do this just to make sure you're overriding the
server's defaults.

If you want all your notices logged into an error file as well as the fatal
errors, and the server defines the default error level as E_ERROR, then you'll
have to override the default error level.

Horrible article.

------
afhof
These signs would be present in almost any crappy software, not just PHP.

------
phoenix24
also, the ones that don't use an ORM, instead go at lengths to invent it.

~~~
jaxn
What about Wordpress? They don't use an ORM do they?

~~~
crikli
I would posit that Wordpress is the wordwide leader in 'crappy PHP software.'

(And I'm _not_ a PHP hater, my company writes 90% of our stuff in PHP, and WP
works great for blogs and brochure sites. But it's nasty underneath that
pretty UI.)

~~~
jaxn
I guess it depends how you measure good software. Wordpress works. Really
well.

~~~
prodigal_erik
Wordpress has such a long history of catastrophic security flaws that it's
become notorious among people (like me) who don't even use it.

~~~
jaxn
I am probably not all the different from you. But I have been running
Wordpress on my own server for years and have never had a problem.

------
subwindow
#1. It is written in PHP.

(sorry, I couldn't resist)

