SitePoint Sponsor

User Tag List

Results 1 to 15 of 15
  1. #1
    Love *********'s Forum ep2012's Avatar
    Join Date
    Aug 2004
    Location
    Toronto, Ontario
    Posts
    848
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    What makes good code

    Hi all,

    I'm going to be hiring a programmer who knows PHP/Drupal, & I want to know what makes good code.

    I always hear different things.

    Some say commenting is a must, others say if the code is good enough, it doesn't matter.

    Thanks


    Michelle

  2. #2
    Follow Me On Twitter: @djg gold trophysilver trophybronze trophy Dan Grossman's Avatar
    Join Date
    Aug 2000
    Location
    Philadephia, PA
    Posts
    20,578
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Readable, maintainable, and reasonably efficient.

    If the code isn't bad, it's better than you'll get out of most web contractors.

  3. #3
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    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

  4. #4
    PHP Guru lampcms.com's Avatar
    Join Date
    Jan 2009
    Posts
    921
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    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.

  5. #5
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    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

  6. #6
    PHP Guru lampcms.com's Avatar
    Join Date
    Jan 2009
    Posts
    921
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by arkinstall View Post
    I agree with alot of the above post, but I do

    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.

  7. #7
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    So you're trying to tell me that (using dummy XML functions):
    PHP Code:
    $List $Document->Append('ul');
    $List->AddAttribute('class''menu');
    for(
    $i 0$i 10$i++){
        
    $Item $List->Append('li');
            
    $Link $Item->Append('a');
            
    $Link->AddAttribute('href''?i='.$i);
            
    $Link->SetText('Item ' $i);
    }
    echo 
    $Document->Render(); 
    Is better than:
    PHP Code:
    <?php
    echo '<ul class="menu">';
    for(
    $i 0$i 10$i++){
        echo <<<ITEM
            <li><a href="?i={$i}">Item {$i}</a></li>
    ITEM;
    }
    echo 
    '</ul>';
    Anyone who knows HTML - which includes the PHP developer, the HTML coder etc - can modify that safely to some extent.

    Now, with the former only the PHP developer can comfortably (if that) edit the HTML. Not really worth the extra typing either.
    Jake Arkinstall
    "Sometimes you don't need to reinvent the wheel;
    Sometimes its enough to make that wheel more rounded"-Molona

  8. #8
    PHP Guru lampcms.com's Avatar
    Join Date
    Jan 2009
    Posts
    921
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Right, that would be hard for non developer/non programmer.

    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');



    Quote Originally Posted by arkinstall View Post
    So you're trying to tell me that (using dummy XML functions):
    PHP Code:
    $List $Document->Append('ul');
    $List->AddAttribute('class''menu');
    for(
    $i 0$i 10$i++){
        
    $Item $List->Append('li');
            
    $Link $Item->Append('a');
            
    $Link->AddAttribute('href''?i='.$i);
            
    $Link->SetText('Item ' $i);
    }
    echo 
    $Document->Render(); 
    Is better than:
    PHP Code:
    <?php
    echo '<ul class="menu">';
    for(
    $i 0$i 10$i++){
        echo <<<ITEM
            <li><a href="?i={$i}">Item {$i}</a></li>
    ITEM;
    }
    echo 
    '</ul>';
    Anyone who knows HTML - which includes the PHP developer, the HTML coder etc - can modify that safely to some extent.

    Now, with the former only the PHP developer can comfortably (if that) edit the HTML. Not really worth the extra typing either.

  9. #9
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Good that we're on the subject of good code then, isn't it

    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

  10. #10
    Love *********'s Forum ep2012's Avatar
    Join Date
    Aug 2004
    Location
    Toronto, Ontario
    Posts
    848
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Just to be clear, I'm talking PHP code here, but thank you.


    Michelle

    Quote Originally Posted by arkinstall View Post
    Good that we're on the subject of good code then, isn't it

    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.

  11. #11
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    So am I.

    My point is that when in PHP, it's much more beneficial (∴ better in terms of code) to output HTML as a string than via an XML processor such as the DOM, as a response to SharedLog - still on topic.
    Jake Arkinstall
    "Sometimes you don't need to reinvent the wheel;
    Sometimes its enough to make that wheel more rounded"-Molona

  12. #12
    Love *********'s Forum ep2012's Avatar
    Join Date
    Aug 2004
    Location
    Toronto, Ontario
    Posts
    848
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks to everyone.

    I really didn't understand most of what was written b/c it was in techi speek, but I got some ideas.

    I've been saying for years, that there should be a service (I'll start the company if anyone is interested) that screens code for companies who are looking to hire.

    I know not everyone agrees about some things, but I would assume most great programmers agree with the meat & potatoes.


    Michelle

  13. #13
    PHP Guru lampcms.com's Avatar
    Join Date
    Jan 2009
    Posts
    921
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    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.

  14. #14
    SitePoint Wizard
    Join Date
    Mar 2008
    Posts
    1,149
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    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.

  15. #15
    SitePoint Guru
    Join Date
    Oct 2006
    Location
    Queensland, Australia
    Posts
    852
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Sharedlog.com View Post
    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...


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •