TalkPHP - View Single Post

Salathe · 12-05-2007, 04:33 PM

Thanks for your post Tanax, it's always nice to see what other people think with regards to "pretty" code. Personally, I think your layout is more difficult to read than my own -- it might be because I use my style day-in-day-out or mine might be quantifiably cleaner, I don't know.

For a start, there should always be a space between an if and the condition. This is simply to a visual difference since the if is a language construct and not function.

Your excessive use of whitespace also aids the relative lack of readability, for me. That might not be what you were expecting to hear but if something takes 20 lines of code (with 10 lines of nothing) where it could just as cleanly (or cleaner!) be written in 10 lines, I'd go for the latter every time.

Something that you didn't mention explicitly, but is apparent in your examples of 'good' and 'bad' code is that comparison/assignment operators (and others) should be surrounded by a space character: true==true versus true == true, $i=0 versus $i = 0.

Finally, your comments. While I'm a huge fan of documenting code -- it's something everyone should do out of habit -- I've got a bone to pick with yours. Particularly with comments along the lines of, "Checking if the function admincheck returns true" placed directly above a very simple, obvious if statement. That comment, and others in a similar vain, is really of no practical use whatsoever. Slightly better in this case would have been "If user is an administrator" because at least that doesn't simply, literally, describe what the line of code below is doing but places it in the context of the rest of the script.

Sorry but I'm really not a fan of your 'good' code style and will need more winning over than is provided above to sway my opinion.

Examples of 'good' and 'bad' code? You decide:

PHP Code:


			
if($_GET['user'] == 'Tanax') {

    // execute tanax :-)

}

else {

    // someone who isn't tanax

}

PHP Code:


			
if ($_GET['user'] == 'Tanax')
{
    // execute tanax
}
else
{
    // someone who isn't tanax
}

PHP Code:


			
if ($_GET['user'] == 'Tanax') {
    // execute tanax
} else {
    // someone who isn't tanax
}

PHP Code:


			
// Spot the difference ;-)
if ('Tanax' == $_GET['user']) {
    // execute tanax
} else {
    // don't execute him
}

You'll notice that I never said which style I use -- I did that on purpose. No favourites here.

12-05-2007, 04:33 PM	#2 (permalink)
Salathe Moderateur Join Date: Apr 2007 Posts: 1,393 Thanks: 5	Thanks for your post Tanax, it's always nice to see what other people think with regards to "pretty" code. Personally, I think your layout is more difficult to read than my own -- it might be because I use my style day-in-day-out or mine might be quantifiably cleaner, I don't know. For a start, there should always be a space between an `if` and the condition. This is simply to a visual difference since the `if` is a language construct and not function. Your excessive use of whitespace also aids the relative lack of readability, for me. That might not be what you were expecting to hear but if something takes 20 lines of code (with 10 lines of nothing) where it could just as cleanly (or cleaner!) be written in 10 lines, I'd go for the latter every time. Something that you didn't mention explicitly, but is apparent in your examples of 'good' and 'bad' code is that comparison/assignment operators (and others) should be surrounded by a space character: `true==true` versus `true == true`, `$i=0` versus `$i = 0`. Finally, your comments. While I'm a huge fan of documenting code -- it's something everyone should do out of habit -- I've got a bone to pick with yours. Particularly with comments along the lines of, "Checking if the function admincheck returns true" placed directly above a very simple, obvious if statement. That comment, and others in a similar vain, is really of no practical use whatsoever. Slightly better in this case would have been "If user is an administrator" because at least that doesn't simply, literally, describe what the line of code below is doing but places it in the context of the rest of the script. Sorry but I'm really not a fan of your 'good' code style and will need more winning over than is provided above to sway my opinion. Examples of 'good' and 'bad' code? You decide: PHP Code: `if($_GET['user'] == 'Tanax') { // execute tanax :-) } else { // someone who isn't tanax }` PHP Code: `if ($_GET['user'] == 'Tanax') { // execute tanax } else { // someone who isn't tanax }` PHP Code: `if ($_GET['user'] == 'Tanax') { // execute tanax } else { // someone who isn't tanax }` PHP Code: `// Spot the difference ;-) if ('Tanax' == $_GET['user']) { // execute tanax } else { // don't execute him }` You'll notice that I never said which style I use -- I did that on purpose. No favourites here.