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.

Good code, for me, is code which is written to such standard that you can always tell what is going on.

Most programmers don't utilise classes and functions as much as they should. If your code requires commenting, you need to refine it. Of course, you can still comment it - but good code doesn't need it.

Jake Arkinstall
"Sometimes you don't need to reinvent the wheel;
Sometimes its enough to make that wheel more rounded"-Molona

I'd say that comments are a must especially if you write an open source project where other people will be using and possibly modifying your code.

But there is so much more to a good code than comments. There are many 'best practices' to follow. For example, make your methods name self-explanatory, like sendRegistrationEmail()

or appendAttachment()

this way it's clear what the method is doing.
Also try to make your methods short and responsible for only a small task. It's better to have many small methods that a few large methods.

Try not to use private methods, use protected instead of private, it's just going to be easier to write test cases for protected methods than for private.

Don't use $GLOBALS array to store your variables.

Don't define constants that are not used all the time. Defining too many constants is generally a bad idea.

Use common patterns like singleton. Singleton is a very good pattern to use for Database classes. You don't have to pass singleton around from object to object, you can always get a copy of an already instantiated object.

Almost always use factory method to instantiate a class instead of __construct(): you can easily modify the factory if you have to. And make your __construct() a protected method.

Always have a good error handling and always have good logging system to log errors, preferably with an ability to send email to developers automatically when error is logged.

Utilize custom exceptions

Utilize SPL objects, the most useful of them would be ArrayObject, you can extend it to fit your needs.

For handling files use SplFileInfo - extend it to fit your needs and then you can also use FilterIterator to filter out the files you need.

There are also some naming conventions for naming your parameters. For example, if a param is going to be an interer, call it $intSomething, a string call $strSomething, etc.

This way you will always know what type of param it is. A shorter convention is to just use 's' prefix for string, 'o' for object.

Try to use PDO for database functions, there are many usefull fetch methods that can return result of fetch as an associative array where the value of first column will become key and value of second column will become value. You can't do this with using just mysql functions.

Try to use DOM class or SimpleXML for generating HTML. This way your html will have much better chance of being valid.

And of cause use objects the right way. Remember that just because the code uses objects does not make it an object oriented program. There is a lot more to object oriented programming that using objects.

I agree with alot of the above post, but I do disagree with some of it:

First of all, singleton definitely causes more problems than it solves, so I would recommend against that but instead recommend the registry approach - similar to singleton but requiring instantiation, allowing for multiple variations in a single request.

NEVER use Dom or SimpleXML for generating HTML. It not only complicates your code, but it slows it down quite considerably.

Variables are auto stored into $GLOBALS, so I don't see how you could avoid that - maybe you're mistaking it with register_globals?

Jake Arkinstall
"Sometimes you don't need to reinvent the wheel;
Sometimes its enough to make that wheel more rounded"-Molona

NEVER use Dom or SimpleXML for generating HTML. It not only complicates your code, but it slows it down quite considerably.

Variables are auto stored into $GLOBALS, so I don't see how you could avoid that - maybe you're mistaking it with register_globals?

I recently started using SimpleXML for generating code and must say that its a lot faster than I could ever expect. I also thought that it would add too much overhead, but to my surprise it actually did not add any extra time comparing to just appending the html string.

I use Xdebug to time all my scripts all the time, so I say this with certainty that SimpleXML is extremely fast for generating code and it does not really make it more complicated, once you get used to it.

I don't use just pure SimpleXML class, what I did was I extended it to use my own methods. For example, SimpleXML returns true for addAttribute(), which is not very helpful, so instead I defined my own method setAttribute and it returns $this, which is yet another tip I can give: if your method does not need to return anything specific, you should return $this. Returning $this is much better than not returning anything or even than returning true/false
because it allows you to use chain pattern.

There are many more methods you can add to SimpleXML.

And about the $GLOBALS, I mean don't use $GLOBALS to store things, like some people like to do this:
$GLOBALS['my_stuff'] = array('item', 'item');

This is not a good idea. Instead its better to use class variables.

And another tip - use XDebug to performance test your script. This can help you find inefficient code that could be improved.

And one more tip: use type hinting, especially when passing objects as parameters. This way its very clear what object it being passed, so another developer will know that a parameter is not only an object but also of what class.

and then set your error handling to turn E_RECOVERABLE_ERROR into custom exceptions. This way type hinting errors will be caught as exceptions and you can just display the error on the screen in an nice way as well as notify developer by email.

As a developer you can write your own methods, so you can add method addTag($tagName, $tagValue) and make it return the newly added simpleXMLelement
you can also write a method addLink($url)
and you can also add method addClass($classname)

then your code will be:

$List = $Document->addTag('ul')->addClass('menu');

for($i = 0; $i < 10; $i++){
$List->addTag('li')->addLink('i='.$i);
}

After awhile even a non-programmer will figure out how to use these

Of if you know that your html tags usually have attributes 'id' and/or class
you will make your addTag method like this:
addTag($tagName, $class = null, $id = null);

Then your first like of code will be even easier:
$List = $Document->addTag('ul', 'menu');

Good code is code which is portable, code which is understandable and code which requires the minimum possible learning curve to edit.

HTML is a language that anyone who will want to edit the file will have an at least basic knowledge of. It's also an efficient language, and next to no processing is required to output a string, whereas a comparatively considerable amount is required to first process the XML THEN output said string.

It adds much more complexity for minimal gain - the output code may be syntactically correct, but that is easy to achieve by hand.

Jake Arkinstall
"Sometimes you don't need to reinvent the wheel;
Sometimes its enough to make that wheel more rounded"-Molona

Good code is code which is portable, code which is understandable and code which requires the minimum possible learning curve to edit.

HTML is a language that anyone who will want to edit the file will have an at least basic knowledge of. It's also an efficient language, and next to no processing is required to output a string, whereas a comparatively considerable amount is required to first process the XML THEN output said string.

It adds much more complexity for minimal gain - the output code may be syntactically correct, but that is easy to achieve by hand.

I know I'm not going to convince people to just start using DOM or SimpleXML to add all their html tags. It's just a way I started doing not long ago and I like it. When you add tags to SimpleXMLelement using addChild(), it adds the data directly to internal data structure in php, so it does not have any overhead of parsing xml string.

Surprisingly this may just be taking just as much time as when you append a string to an existing string using the . (dot) in php.

The DOM class may be more resource intensive, but SimpleXML is not DOM, and it's very fast.

Ok, the reason I am using SimpleXML is because I am often add non-html tags because I am generating a lot of XML as the output.

Code is one thing, but I'd be more comfortable if I knew the person had some breadth knowledge. Like know something about the HTTP protocol, has set up a web server before, know even basic (computer science) data structures (although highly useless with PHP), etc.

A lot of people know PHP and JavaScript well, but they won't be able to make anything like Google Maps, because it'd be slow, bloated, and crappy. All the breadth knowledge is needed to solve problems well.

For example, if you were to write an Ajax chat client, if you had written GUI programs before (lower level), you know that you have to purposely redraw the screen only when you needed to. That, when applied to making pages, lets you know that updating the DOM constantly with visible elements means that the browser has to redraw every time you do it (since you never tell it when to explicitly redraw), and it would be obvious that you need to find a way to update the DOM in a way where the browser doesn't have to redraw the screen until the last moment. Someone without that knowledge is going to make one crappy and slow interface. (Though in the last few years, since web development has gotten a bit more popular, people have written on this subject, so now you can actually learn it from someone else. But you get the idea.)

People who know breath knowledge also usually are genuinely interested in the subject and probably know it well. It just tends to be that way with anything, be it programming or mathematics.

Try to use DOM class or SimpleXML for generating HTML. This way your html will have much better chance of being valid.

A better change of being valid? If you don't know HTML well enough to be able write valid code, then I think you should focus developing your skills in that area rather than using an XML/HTML generator such as SimpleXML. It just doesn't make any sense...