The SitePoint Forums have moved.

You can now find them here.
This forum is now closed to new posts, but you can browse existing content.
You can find out more information about the move and how to open a new account (if necessary) here.
If you get stuck you can get support by emailing forums@sitepoint.com

If this is your first visit, be sure to
check out the FAQ by clicking the
link above. You may have to register
before you can post: click the register link above to proceed. To start viewing messages,
select the forum that you want to visit from the selection below.

How to determine code quality in PHP

Following up on the "utter disgrace" thread, how about discussing what kind of code we want to see in open source projects. Mostly I think I know good code from bad when I see it, but my criteria might be idiosyncratic. I seem to remember selkirk posted something relevant to this a while ago, but I couldn't find anything in a quick search. Also, this is a relevant objection:

Originally Posted by CapitalWebHost

I see a lot of postering in this thread...most of i comes of as primadonna type of code bashing.

Instead of just bashing code ramdomly, why not explain to others reading this thread that are not "God Gifted Programmer" why the code is bad?

I tend to look for the following:

Classes. "The more, the better" may be a stupid over-simplification, but my impression is that with existing projects, it's a pretty good indicator of quality. And up to a certain point, it is the inverse of the "large class" code smell.

Unit tests. The few projects who actually have unit tests are clearly likely to have better code. Of course, the next criterion will be actual coverage, as opposed to assertEqual(2+2,4)

No deep nesting of conditionals and loops. To a certain extent, this may be a matter of taste, but I would think that anything more than two levels is a negative indicator, since it's likely to be hard to unit test.

Naming. For example, if the average length of variable names is less than 1.5, there's a problem.

This is just my personal list off the top of my head. I know there are fancier, more academic criteria as well. And there's security, which is a separate set of concerns, but obviously important.

It is a well-known truism that good code is like a good prose. Everyone can see when something is going wrong, but there is no cookbook recipe on how to make it good. A good style is mostly a matter of taste. I'd arrange things like this:

- Frugality.*) If you can get along without something, remove it.
- Conciseness. No methods longer than 10 lines, no classes with more than 10 methods.
- Uniformity. No matter what conventions you're using, be consistent.

I'll pretty much sum up with what stereofrog is proposing. Besides that, I personally wouldn't care much if it is OOP or procedural, as long as it is well structured and thought off. Unit tests are not relevant to me but a clear separation of concerns, as dagfinn indicates, and a well applicated principle of single responsability (yes, that's also appliable to libraries) do miracles.

I also like clean code. Whether that implies a max of 10 lines per method/function or methods per class, I'll leave it open; I'm not too fond of hard requirements, but it has to be lean.

Another issue I'll take into account is security. Please, no register_globals or the like. No accessing of request variables directly and without checks. No session ids on an URL. No saving data in the DB without first CHECKING ...

And as a general rule of thumb, I don't want, as a developer, to have to spend more than 10 min. looking at source code to be able to figure out how an application functions. Having to open 10/15 different files in order to follow the course of a variable makes me really sick.

If in addition, the application is built in a fashion that allows me without too much hassle to add or remove features, you've probably got a fan.

EDIT .- Forgot to mention coding standards. Embrace them, whichever you may like the most, and please STICK to them.

You can make a quick list from the Refactoring book code smells. If nothing else, it shows that the author has read the thing. I would add unit tests as well, especially for "frameworks" and libraries. It is highly likely I will want to patch the code at some point.

Even before looking at the code though, an easy install is a good indication. It shows that the developer is thinking about their users.

My biggest requirement though, is readable code. This needs breaking down further of course. I want to read a code snippet or class signature and know what it does. I want to know this without looking at the code underneath. This cuts it...

It's not clear where I put my code for each of these methods. The developer did not put themselves in my shoes, as that will be my first question.

This means...

1) Good naming. Both snippets are OK here.
2) Precise methods.
3) Explicit parameter passing. No mystery, behind the scenes, sniffing out of information.
4) I should not be required to contruct more than a handful of objects.

I don't think we can come up with more than a bunch of heuristics for this problem, but it's an important question. As is the associated question - "what makes a good programmer?".

I actually can't stand seeing a class, where it's obvious that it was used just for the sake of being OOP. Consistency is really nice. Correctly named variables, classes, methods and functions. I want to be able to look at the code and understand what's going on. I've seen simple sites use nothing but gobs of classes/objects and it's crazy to me. Code should be obvious and free of ego. Simple and concise. Sometimes a neat way of doing things is just that... "neat". But not the best or most simple way. When I code now, I'm always thinking, "Would I want someone else to see this in a year from now? Would I and/or anyone else understand what's going on?". Also, NO extra features when they aren't needed! Comments are wonderful when they are simple and give an example. A README.txt file is also great for general notes about the application or site. Sometimes the code can look insane, but an explanation can help get you into the correct way of looking at it. Sorry for the mumble jumble!

All good points. I go for comments when I have to explain why I'm doing something that cannot be conveyed from the syntax alone - business logic rules. The rest of the time I try to make my code succinct but convey what it is doing by use of properly named methods and variables.

My boss is obsessed with performance and will not exceed 8 characters for a variable name (querresu) - this drives me nuts! Coupled with his love of variable variables you can imagine how easy this stuff is to debug.

I try live by a quote from Martin Fowler's Refactoring when coding:

Any fool can write code a computer can understand. Good programmers write code humans can understand.

- Human readable code. You won't be maintaining that code for the rest of your life, so make it easy for other people to understand.
- Ego free code. I don't care how smart you think you are, there's someone out there smarter.
- Smart use of design patterns. Use them only when necessary. Throwing them in when they're not really useful is just stupid.
- Good object oriented principles when necessary.
- Following from the above, not using OO when it's not necessary.
- Good method naming. Make them descriptive, but don't make them too long.
- Consistency in variable names, method names, CaSiNg, etc.
- Good use of comments. I don't want to drown in a sea of green...
- Avoiding as many Code Smells as you can.

When people open your code in a text editor the first thing they look at is the cleanliness and structure. If it looks like crap I bet they won't think too highly of you (I know I wouldn't ). I don't really ask for much...

When we talk about OOP and PHP you need to mention what version of PHP we are talking about (as we all know PHP4 is very limited when it comes to OOP). PHP5 on the other hand has a lot of features that help OOP. For example, which is better to understand when you see:

PHP Code:

function __my_function() { }

or

PHP Code:

private function my_function() { }

On the behalf what makes quality code here are my top 3 in the order of importance:

Inline Documentation (it could be only personal preference but I prefer inline documentation than a manual or some other document outside the code that explains methods, and classes)
Following Basic Naming Conventions (for example, start each function name with a verb, keep your variable names between 4-8 characters, etc)
Divide and conquer (don't create classes with 3 functions each 40 lines long, try to separate it logically)

But if the cat has cancer, or a brain tumor and it dies the next day... then what?

I like this point of view. Optimism is a must for a good PHP developer.

Good PHP code practices. Hm:

1. Comment. After you open your project in several months, you start guessing, what is what. So comment thoroughly. Don't leave commenting for the end, when the application is ready.

2. Test. Test the application thoroughly before distributing it to the web.

3. Do not use databases. . Well, I see, most of the developers, are to disagree. So let's periphrase it. Use databases, only for projects where data is very sensitive, changes fast, regularly, and is large. So what is large? It's a relative idea. One may say 12 MB, 700 MB or 4 GB. According to my experience 99% of all the projects don't need (MySQL,Acess - like)databases. "Please, excuse us but the MySQL server is not working a the moment", "The maximum connections exceed the limitations of the MySQL server", etc.

And the cat sleeps, dreaming of more mice to come, for she had eaten them all!

Interesting that many people are putting so much emphasis on commenting - I've discussed this in general development before, but surely clear, self-documenting code whose intent is made clear in both the code itself and its accompanying unit tests is better than what is often pretty pointless documentation. Too often you see comments along the lines of:

Commenting is useful when trying to describe a particularly strange design decision, or a choice of algorithim or other individual cases but often lots of comments seem like a bad code smell to me - ask yourself, if I have to write a comment to make the intent clear, then surely my code could be clearer in the first place?

To take an example from a project I'm currently working on (RubyOnRails project), none of our controllers have comments except in a few places. The names of the controller actions and the code itself make the intent pretty clear, but if I want to see what the controller is doing, I can simply look at the unit tests, like these, for a controller action that displays a list of fees (its important to note that in a controller, a unit is generally a single action and not the entire controller - further more I take the Behaviour Driven Development style of test cases per context):

And thats just a small snippet for this particular controller. Without pasting the rest of the tests code for privacy reasons, heres the rest of the agile dox generated JUST for this controller's suite of tests:

New fee screen should:
* return successful response
* display new fee form
* provide list of organisations for form
* provide list of feetypes with blank default for form
* provide an option to create another fee for the same organisation
* provide a link back to fee listing

Now, that to me gives much more value than lots of comments scattered throughout the code, and the above specifications are runnable which means I can make sure of not only what my app does, but if its doing what it should be.