2
\$\begingroup\$

I've designed the basics of an admin page I'd like to use for a small website. I'd like to know if I'm using the language properly. Please rip apart the code and tell me what I could do better.

Admin page code, file called "index.php" (just the file name in my "netbeans" folder):

<!DOCTYPE html>
<html>
<head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
    <link rel="stylesheet" type="text/css" href="admin.css">
    <title>Admin Page</title>
</head>
<body>
    <div>
      <table style="width:100%">
        <td valign="center" width="25%"><img src="BSQ_Logo.jpg" width="61"  height="36"></td>
        <td valign="center" align="center" width="15%"><b>Total Accounts:</b></td>
        <td valign="center" align="center" width="15%"><b>Logged On:</b></td>
        <td valign="center" align="center" width="15%"><b>Last Updated:</b></td>
        <td width="25%">
            <ul id="navlist">
                <li><a href="#">Item 1</a></li>
                <li><a href="#">Item 2</a></li>
            </ul>
        </td>
      </table>  
    </div>
    <br />
    <div>
        <table id="labels" border="1" style="width:100%">
            <td width="20%">User</td>
            <td width="10%">ID</td>
            <td width="10%">Listings</td>
            <td width="10%">Connections</td>
            <td width="5%">Flags</td>
            <td width="10%">Days</td>
            <td width="25%">Email</b></td>
            <td width="10%">Suspend</td>
        </table>
    </div>
    <?php
    include 'data_sheet.php';

    // call funciton form data_sheet.php to get array of data table
    $user_data = load_data($r);

    // array to line up the cells in the tables
    // each row under the collumn headings will be it's own table
    $cell_width = array('20%','10%','10%','10%', '5%', '10%','25%','10%');

    $listings_array = array(
        array('8343','7410', '1352'),
        array('6400','1432'),
        array(),
        array('3721','0185','4903','2132')
        );


    // funciton to that will return the a dropdown mennu
    function drop_menu ($listings)
    {
         $n_of_items = count($listings);
         echo "<form><select>
               <option>" . $n_of_items . "</option>\n";
         foreach ($listings as $key => $element)
         {
             echo "<option>" . $element . "</option>\n";
         }
         echo "</select></form>";
     }  // end of drop_menu function

     // function 'create_row creates a table for each row
     /*
      * aruments need to be:
      * --$prime coutner counts which row we are in
      * --$element i.e. array of info for each cell, which 
      *    should be the sub array from the database call
      * -- $cell_width from the cell width formatting araay
      * -- an array for each dropdown menue
      *    starting with 'listings' and 'connections'
      * -- $cell_width array - for formating each cell
      */
     function create_row ($prime_counter, $element, $listings, $connections, $cell_width)
     {
        // $sub_counter is for keeping track of which cell 
        // in the row is being dealth with
        $sub_counter = 0;
        // being the table for the account - one account/table per row
        echo <<<EOT
<br /><div><table border="1" style="width:100%">
EOT;
        // primary foreach loop goes through each 
        // account and lays out data in each cell;
        // data form a call to teh database
        foreach ($element as $subkey => $sub_element)
        {

            // begin cell in table
            echo <<<EOT
<td width=$cell_width[$sub_counter]>
EOT;
            // this determins what goes in the cell
            switch ($sub_counter) 
            {
                // case 2 is the listing information
                // need code here to putt the correct array
                // from the array of array of listings
                case 2:
                    // drop menu is to show a list of listings
                    // in the cell under the listings column
                    drop_menu($listings[$prime_counter]);
                    break;
                // default is the data from thte datbase call
                default:
                    echo $sub_element;
                    break;
            }
            // end cell in table
            echo "</td>";
            $sub_counter++;
        }// end foreach sub
        // close the table
        echo <<<EOT
</table></div>
EOT;
     } // end create row function

// ****begin display code for the tables under the column headings **********************

    /* foreach loop that puts data into the tables 
     * under the column headings
     * one table for each row
     * $user_data is the two dimentional array from the dataase
     */
    foreach ($user_data as $key => $element)
    {
        // need a database call based on the user data to get the
        // list of connections and listing for each user

        // $key to trigger the right arugments for
        // each row
        create_row($key, $element, $listings_array, 0, $cell_width);   
    }// end foreach main

    ?>
</body>
</html>
\$\endgroup\$
2
  • 2
    \$\begingroup\$ At first glance, I don't see anything "wrong" with this. However, have you considered separating the logic from the view i.e. using the MVC pattern? \$\endgroup\$
    – karancan
    Mar 24, 2013 at 16:03
  • 1
    \$\begingroup\$ You are using tables as layout/design. Cascading StyleSheets (CSS) is for layout. \$\endgroup\$
    – MECU
    Mar 24, 2013 at 17:02

1 Answer 1

2
\$\begingroup\$

PHP Related Things

  • Try not to mix HTML and PHP. PHP logic should come on top of the document, HTML below it. Within the HTML, there are just some minor echo and if statements and some loops. If you go a little bit more advanced, you should use something like the MVC pattern (like @karancan suggested).

Tips

  • Take a look at PHPdocs. That's a common used way of documenting you PHP code.

HTML/CSS Related Things

<td valign="center" width="25%"><img src="BSQ_Logo.jpg" width="61"  height="36"></td>

You should not use inline HTML (e.g. width=, height=, valign=). This should be done with CSS in a stylesheet or within a <style> element.


There is not much more to say about this script. You are now using functions. I suggest to take a look at Object Oriented Programming (OOP) if you are a little bit more familier to PHP.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.