SitePoint Sponsor

User Tag List

Results 1 to 3 of 3
  1. #1
    Bah, I'll just hack it DoobyWho's Avatar
    Join Date
    Jul 2002
    Posts
    476
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    What's wrong with this code?

    My boss gave me this code that an applicant gave him. He wants me to write up a little email to him about what is wrong/shouldn't be done, or about what i just dont like about the code.

    What do you guys see about it ?

    script 1:

    PHP Code:

    <?php
        $link 
    mysql_connect("sitesite.org""user""pass");
        
    mysql_select_db("data");

        
    $query "SELECT * FROM Meeting_Summary";
        
    $result mysql_query($query);

        print 
    "<table>\n";
        while (
    $line mysql_fetch_array($resultMYSQL_ASSOC)) {
            print 
    "\t<tr>\n";
            foreach (
    $line as $col_value) {
                print 
    "\t\t<td>$col_value</td>\n";
            }
            print 
    "\t</tr>\n";
        }
        print 
    "</table>\n";

        
    mysql_free_result($result);
                
        
    mysql_close($link);
    ?>
    script 2

    PHP Code:

    <?php
        $link 
    mysql_connect("sitesite.org""user""pass");
        
    mysql_select_db("data");

        
    $query "SELECT * FROM QandA";
        
    $result mysql_query($query);

        print 
    "<table>\n";
        while (
    $line mysql_fetch_array($resultMYSQL_ASSOC)) {
            print 
    "\t<tr>\n";
            foreach (
    $line as $col_value) {
                print 
    "\t\t<td>$col_value</td>\n";
            }
            print 
    "\t</tr>\n";
        }
        print 
    "</table>\n";

        
    mysql_free_result($result);
                
        
    mysql_close($link);
    ?>
    The would be running on a site called 'sitesite.org' so the first thing i would think would be that the guy is using 'sitesite.org' as the server instead of using 'localhost'

  2. #2
    SitePoint Wizard siteguru's Avatar
    Join Date
    Oct 2002
    Location
    Scotland
    Posts
    3,631
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    You can't berate an applicant for not knowing the DB connection parameters if he/she wasn't told them. Whilst it is usually localhost as the server there is no guarantee of that.

    It depends on what the applicant was asked to do? In terms of functionality, the scripts shown will display the full contents of each DB table noted as HTML tables, and the HTML source will be well-formatted and easy to read.

    The main thing I see is this ... if anything goes wrong with the connection, DB selection or queries then the script will break - there is no graceful degradation, it either works or it doesn't.

    The other main thing is that the mysql_close() line is superfluous - the connection is dropped when script execution finishes anyway. This command is only relevant if you do some DB manipulation then have a LOT of other code to process before the script finishes - in that case it may be worth having that line in.

    Finally, you should avoid using SELECT * whenever possible - it is better to define the fields you want (even if it is all of them) as this is usually executed faster by the DB server.

    I'm sure others will have further comments on how to rationalise the code or make it more elegant, but that's it from me.
    Ian Anderson
    www.siteguru.co.uk

  3. #3
    Bah, I'll just hack it DoobyWho's Avatar
    Join Date
    Jul 2002
    Posts
    476
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    He does know the DB connection parameters except for the l/p. But I see what you're saying about the rest of it.


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
  •