TalkPHP
 
 
Account Login
Latest Articles
» The basic usage of PHPTAL, a XML/XHTML template library for PHP
» Vulnerable methods and the areas they are commonly trusted in.
» Simple way to protect a form from bot
» The Basics On: How Session Stealing Works
» How to keep your forms from double posting data
IRC Channel
IRC Speech Bubble Join the friendly bunch on IRC...
(#TalkPHP on Freenode)

...Also available via a web interface.

See this thread for information on the TalkPHP Free Hugs Initiative™. Subject to availability.
Associates
Associates
CSS Tutorials
Reply
 
LinkBack Thread Tools Search this Thread Display Modes
Old 12-05-2007, 03:36 PM   #1 (permalink)
The Prestige
Upcoming Programmer Inquisitive 
 
Tanax's Avatar
 
Join Date: Sep 2007
Location: Sweden, Stockholm
Posts: 1,080
Thanks: 115
Tanax is on a distinguished road
Default Coding nicely

I've seen alot of people's codes here on talkPHP, and I thought I'd write an article about how to code nicely.

Let's start with just some basic stuff.

if- and else statements

Let's just code a really simple if statement:
php Code:
if($_GET['user'] == 'Tanax') {

}

As you see, it's really simple, and you might think that you already know how to code an if statement.
But I've seen some who codes like this:

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

}

The example above, is ofcourse by all means, NOT incorrect. It will work the same.

However if you sometimes want to edit your code, or upgrade it, or perhaps post it here for feedback, it's much harder to read.

This is, ofcourse, user preference, but as you read more in this article, you will perhaps understand why I code the way I do, and why I think that the above example isn't the best way to go.

Now, let's do something inside our if statement:

php Code:
if($_GET['user'] == 'Tanax') {
   
    if(admincheck('Tanax') == TRUE) {
       
        if(isset($_POST['submit'])) {
           
            // Do something..
           
        }
       
        else {
           
            // Do something else..
           
        }
       
    }
   
    else {
       
        echo 'You are not an admin.';
       
    }
   
}

else {
   
    echo 'Select a user.';
   
}

I have.. NO idea what this script would do.
But this is the blueprint. As you see, I'm placing the starting bracket, always directly after the if statement, on the same line. The closing bracket, is places directly under the if statement, that way you know which bracket closes which if statement.

The else statements, you can do pretty much how you want, but this is how I prefer it. Some may do like this:

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

} else {

}

And I think it looks just as good. However, I prefer the way I did in the previous example.


Another thing you might've noticed, was that I always did one line spacer between the if statement line, and the code inside the brackets. Also, I did a TAB, so I know that the code that are one TAB to the right-> is inside the first if statement.

It's easy to keep control of your script, and easy to go back and make changes to it!


Now, one really important thing, is that you should ALWAYS write comments. Wheather it's a really simple thing that seems obvious at the time. Because 1 month later, you might realize that you want to make a class out of your gallery script(for example), and that way, the comments can help you remember why you did that, and what it did.

Let's go back to our script, a good script would look something like this:

php Code:
// Checks if the user in the URL is set to Tanax.
if($_GET['user'] == 'Tanax') {
   
    // Checking if the function admincheck returns true.
    if(admincheck('Tanax') == TRUE) {
       
        // If the submit button has been pressed.
        if(isset($_POST['submit'])) {
           
            // Do something..
           
        }
       
        // If the submit button haven't been pressed.
        else {
           
            // Do something else..
           
        }
       
    }
   
    // If the admincheck function did not return true.
    else {
       
        echo 'You are not an admin.';
       
    }
   
}

// If the user in the URL is not set.
else {
   
    echo 'Select a user.';
   
}

Yes, it does generate more lines. Even for this simple script. But in the end, you will benefit greatly from it!

Now just to proove my point, here's also an example of a bad code:

php Code:
if($_GET['user']=='Tanax')
{
if(admincheck('Tanax')==TRUE)
{
if(isset($_POST['submit']))
{
// Do something..
}
else
{
// Do something else..
}
}
else
{
echo 'You are not an admin.';
}   
}
else
{
echo 'Select a user.';
}

As you see, it's hard to keep track of which closing bracket is closing which statement.

And if you would look back at this script 2 months after you've written it, it would take a good 10 minutes to figure out what all that does.

Just compare the example of the "good" code I wrote above this example, with the example of "bad" code I wrote in the example above.


This isn't an article to point out that "you code wrong". Because I hope you don't misunderstand me. I'm just pointing out ways for you to code so it's easier to read.

Happy coding!
// Tanax
Tanax is offline  
Reply With Quote
Old 12-05-2007, 04:33 PM   #2 (permalink)
Moderateur
RegEx Guru PHP Guru Top Contributor Advanced Programmer 
 
Salathe's Avatar
 
Join Date: Apr 2007
Posts: 1,393
Thanks: 5
Salathe is on a distinguished road
Default

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.
Salathe is offline  
Reply With Quote
Old 12-05-2007, 05:19 PM   #3 (permalink)
The Contributor
Upcoming Programmer 
 
Matt83's Avatar
 
Join Date: Oct 2007
Location: Argentina
Posts: 72
Thanks: 18
Matt83 is on a distinguished road
Default

Thanks Tanax for this post, I tend to see and process inside these different alternatives as i might be missing a technique or concept that could improve my workflow, as an example this post changed the way i write my variables, but back on your post i can say that i dont think the examples mentioned above qualify as bad code. This is like saying that coke is better than pepsi or the other way around.
Anyways, for me now a days i found this style to work much "better":

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

__________________
http://www.mattvarone.com
Matt83 is offline  
Reply With Quote
Old 12-05-2007, 05:20 PM   #4 (permalink)
The Prestige
Upcoming Programmer Inquisitive 
 
Tanax's Avatar
 
Join Date: Sep 2007
Location: Sweden, Stockholm
Posts: 1,080
Thanks: 115
Tanax is on a distinguished road
Default

Thanks for your comment!

Yes, indeed, this is always up to yourself, and what you personally think looks better. It's all a matter of user preferences

@above- No ofcourse not. I actually put it as "bad" and "good" code, and not like bad and good code. (note the " signs).

Tanax is offline  
Reply With Quote
Reply



Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On


All times are GMT. The time now is 06:53 AM.

 
     

Powered by vBulletin® Version 3.6.8
Copyright ©2000 - 2013, Jelsoft Enterprises Ltd.
Search Engine Optimization by vBSEO 3.1.0
Inactive Reminders By Icora Web Design