Good coding practice

Question

I am graduating this June, so before that I want to prepare myself for working in the industry :)

My Question:

For example I want to list all the songs in the table using a list box and this is how I implemented it:

playlist.php

<tr id='' >
<td  width="" class="">&nbsp;&nbsp;<font color=#00000 />Select Songs</td>
<td width="" colspan="">
<select size='25'multiple='multiple' id="select_songs" name="playlist_songs[]">
<?php
display_songs_list();
?>
</select></td>
</tr>

and I have a seperate php_functions.php file where I have implemented all the php functions

function display_songs_list(){
    $query = "select * from songs order by ID asc";

    $result = mysql_query($query);
    if(!$result)
    echo "<script language = 'javascript'>alert('$result Sorry couldn't connect to the database...');</script>";
    else{
        $num_rows = mysql_num_rows($result);
        if($num_rows > 0){
            while($row = mysql_fetch_array($result)){
                $ID = $row['ID'];
                $title = $row['title'];
                $value = $ID.'_'.$title;
                echo "<option id=\"$ID\" value=\"$value\">$ID : $title</option>";
            }
        }
    }
}

Is this way of implementation recommended. What else I can do to increace the scalability/maintainability/re-usability. please guide me on this. Is it recommended to follow any industry coding standards, if so what you perfer. Thank you.

Answer 1

• Inline style attributes (color=#00000) are deprecated and kill kittens. Learn about CSS.
• Tables are discouraged for anything except tabular data (think spreadsheets), use CSS for layout instead.
• Hardcoding whitespace with   is not desirable either if it doesn't add any meaning, learn to add spacing using CSS.
• echoing either HTML or Javascript from the same function is bad. The Javascript alert will cause invalid HTML syntax at the point you're calling the function.
• Mixing database calls and HTML so tightly is not good. Look into MVC separation.
• Outputting a random, rather meaningless Javascript alert to the user in the middle of a half finished page is bad. You should display a dedicated error page instead. See MVC, which helps you accomplish this.
• Mixing single quotes and double quotes for the attributes is inconsistent and makes the code more difficult to read. Stick to one type of quotes instead.
• There is no form in your markup shown to submit the selected select options