php crew, I need help again :o

Discussion in 'OT Technology' started by Leb_CRX, Apr 21, 2006.

  1. Leb_CRX

    Leb_CRX OT's resident terrorist

    Joined:
    Apr 22, 2001
    Messages:
    39,994
    Likes Received:
    0
    Location:
    Ottawa, Canada
    I havent programmed in a while, and when I did it wasn't web programming

    either way my program is pretty simple, gonna have a list of customers, people can select which one, view content entered and add content

    right now the index page queries the DB (all connection info resides in config.php) for the tables (each customer has it's own table) and then displays the list...user selects which customer they want using a radio button, then submits it...page calls itself, checks if submittion is valid (they selected a radio button) if they did starts a session and stores customer, if they did not, asks them to select customer and go back

    Code:
    <?php 
    require 'config.php'; ?>
    
    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml">
    <head>
    <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
    <link rel="stylesheet" type="text/css" href="style.css">
    <title>Untitled Document</title>
    
    </head>
    
    <body>
    
    <?php
    if (!isset($_POST['submit'])) { // if page is not submitted to itself echo the form
    
    $result = mysql_list_tables($db) or die("Could not list tables<br>".mysql_error());
    ?>
    
    <form method="post" action="<?php $PHP_SELF;?>"> 
    <table>
    <tr>
    <td>Select Customer:</td>
    </tr>
    
    <?php
       while ($row = mysql_fetch_row($result)) {
    	echo "<tr bgcolor=\"#CCCCCC\">";
    		echo    "<td>";
               print "$row[0]\n";
    		echo    "</td>";
    
    		echo    "<td>";
    			echo "<input name=customer type=radio value=$row[0] />";
    		echo    "</td>";
    	echo "</tr>";
       }
    ?>
    
    </table>
    <br />
    
    <input type="submit" value="submit" name="submit">
    </form> 
    
    <?php 
    mysql_free_result($result);
    }
    else
    {
    	if (!empty($_POST['customer']))
    	{
    	session_start(); 
    	$_SESSION["table"] = $_POST['customer'];
    	echo ("Client ".$_POST['customer']." Selected. <br> Click <a href=test.php>here</a> to continue<br><br>Click <a href=".$PHP_SELF.">here</a> to go back");
    	}
    	else
    	{
    	echo ("No customer selected. Please select customer.<br><a href=".$PHP_SELF.">Back</a>");
    	}
    }
    ?> 
    
    
    </body>
    </html>
    
    config.php
    Code:
    <?php
    $username = "whateva";
    $password = "whateva";
    $hostname = "localhost";
    $db 	  = "maintenance";
    
    $dbh = mysql_connect($hostname, $username, $password) or die("Unable to connect to MySQL.<br>Please contact System administrator");
    
    mysql_select_db($db) or die(mysql_error()); 
    
    ?>
    
    
    is this ok? is this bad? what can I do to improve? this is my first php projet, usually I just clean up code
    :bigthumb:
     
  2. kingtoad

    kingtoad OT Supporter

    Joined:
    Sep 2, 2003
    Messages:
    55,923
    Likes Received:
    11
    Location:
    Los Angeles
    I prefer separating the HTML from the PHP. Using a templating engine like smarty and using the Pear classes. If you introduce object oriented programming into your code it will also clean up a lot of the mess involved.

    I tend to code a bit different. I write all my PHP code before I parse -any- HTML. Then I call the variables I need in HTML. (Also, if I do this, I won't have any output problems using header(), which I use a lot).

    For example:

    PHP:
    <?php
    require_once("classes/formHandler.php");
    require_once(
    "classes/mail.class.php");

    $connection mysql_connect('...''...''...');
    if (!
    $connection) {
        
    $dBerror[] = "Could not connect to database: ".mysql_error();
    }
    $selection mysql_select_db("wessman_ir"$connection);
    if (!
    $selection) {
        
    $dbError[] = "Could not select database: ".mysql_error();
    }

    if (isset(
    $dbError))
    {
        foreach (
    $dbError as $ohnoes)
        {
            
    $someVar .= $ohnoes.'\n';
        }
        exit;
    }

    $mail = new Mail;
    $form = new formHandler;

    // all other coding necessary, etc
    ?>


    <html>
    ...
    ...
    <span style="color: red;">$someVar</span>
    ...
    ...
    </html>
    This was actually just cut and pasted from a project I'm working on right now. Slightly modified to show what I mean.
     
  3. Leb_CRX

    Leb_CRX OT's resident terrorist

    Joined:
    Apr 22, 2001
    Messages:
    39,994
    Likes Received:
    0
    Location:
    Ottawa, Canada
    ok thanks toad, I see the difference, I will try to start doing that, just confusing as it stands now cause I am a newb

    but other then that, am I doing anything 'wrong' in my code? or is it as long as it works, it works? just dont wnat to catch bad habits early on

    like for example, using client names as tables? or maybe selecting them using radio buttons? or my methods of doing things (besides the splitting up of html and php)
     
  4. Leb_CRX

    Leb_CRX OT's resident terrorist

    Joined:
    Apr 22, 2001
    Messages:
    39,994
    Likes Received:
    0
    Location:
    Ottawa, Canada
    actually another things are 'sessions' really that easy? you just start it, add things to it and kill it if you want?

    what happens if the user closes window? does it die? or does it timeout eventually? just not sure how it works
     
  5. kingtoad

    kingtoad OT Supporter

    Joined:
    Sep 2, 2003
    Messages:
    55,923
    Likes Received:
    11
    Location:
    Los Angeles
    Well, I didn't spend too much time looking over your code but I did see one thing that I would probably take a different approach to... many people fail to utilize this, actually.

    Say if there was a form and I were to press submit, you execute your code upon submission, in order to do that you have to check to see if the submit button was actually pressed by checking it's variable.

    PHP:
    if (isset($_POST['submit'])) {
    // begin pnrocessing
    }
    Which is good that you're using isset, but ideally anyone could set that variable to whatever it wants to be. It's a security issue. I can pass the $_POST['submit'] variable to whatever value I want to, and it would execute your code because the value is set. I prefer doing it this way:

    PHP:
    if (isset($_POST['submit']) && $_POST['submit'] == 'Some Specified Value')
    {
    // Execute your code
    }
    This way your code will only be executed IF the value you specified returns true while testing the $_POST['submit'] var.

    In some circumstances it wouldn't matter, but it is something that should be looked into. There are a lot of forms on the web that are exploitable because of this issue... it's not a bug, it's just lack of control most of the time. This example can be applied to a lot of submission methods used in PHP. Just think security when you're developing.
     
  6. kingtoad

    kingtoad OT Supporter

    Joined:
    Sep 2, 2003
    Messages:
    55,923
    Likes Received:
    11
    Location:
    Los Angeles
    Sessions are pretty easy in my opinion. When your first learning it can be intimidating but after a try it's a really easy concept to grasp.

    Sessions are killed when the browser is completely closed I beleive. Or when you set it... it also times out after a certain amount of time, but I don't know what it exactly is.
     
  7. Leb_CRX

    Leb_CRX OT's resident terrorist

    Joined:
    Apr 22, 2001
    Messages:
    39,994
    Likes Received:
    0
    Location:
    Ottawa, Canada
    ok cool, thanks toad
     
  8. kingtoad

    kingtoad OT Supporter

    Joined:
    Sep 2, 2003
    Messages:
    55,923
    Likes Received:
    11
    Location:
    Los Angeles
    No problem. :)
     
  9. Voltaire

    Voltaire New Member

    Joined:
    Aug 16, 2005
    Messages:
    2,378
    Likes Received:
    0
    Location:
    Bay Area, CA
    a table for each customer. sounds pretty inefficient. how many customers is your site going to have?
     
  10. Leb_CRX

    Leb_CRX OT's resident terrorist

    Joined:
    Apr 22, 2001
    Messages:
    39,994
    Likes Received:
    0
    Location:
    Ottawa, Canada
    30+ but it will be growing/shrinking constantly

    I just figured this would make sense, and would help split them up

    how else should I do it?

    thanks
     
  11. kingtoad

    kingtoad OT Supporter

    Joined:
    Sep 2, 2003
    Messages:
    55,923
    Likes Received:
    11
    Location:
    Los Angeles
    How much data is each customer going to have?

    I'd have a table for all customers, a table for the relating attributes, and a transaction table to bring those both together.
     
  12. Leb_CRX

    Leb_CRX OT's resident terrorist

    Joined:
    Apr 22, 2001
    Messages:
    39,994
    Likes Received:
    0
    Location:
    Ottawa, Canada
    each week at least 1 entry will be added to the customer data, from 0 (when it goes live)

    the only thing is, the method you're describing (and maybe I am not understanding it), if I take out a customer from the customer table, won't his entries remain inside the other tables and create dead data?
     
  13. kingtoad

    kingtoad OT Supporter

    Joined:
    Sep 2, 2003
    Messages:
    55,923
    Likes Received:
    11
    Location:
    Los Angeles
    Ok, here's an example of what I mean. You could actually probably get away with one table, but I like to split it up into multiple tables because it will query the database faster if you're dealing with a large amount of data. Ever try querying a database for a single table that contains massive data? :mamoru2: Not pretty.

    I think this is a better method than creating a table for each individual customer. However, I probably wouldn't approach it if you're not dealing with a lot of data. I won't be dealing with a lot of data in this example.

    customer table
    --------------------------
    CID (Auto incrementing primary key)
    CustName

    customerdata table
    --------------------------
    CustDID (Auto incrementing primary key)
    RegDate
    BirthDate
    CustType

    transaction table
    --------------------------
    TID (Auto incrementing primary key)
    CID (Foreign key referencing primary key in customer table)
    CustDID (Foreign key referencing primary key in customer data table)

    When you query the database you'll be pulling the data from the customer and customer data table where the transaction table is referencing.

    Ex:
    Code:
    SELECT `customer.CustName`, `customerdata.RegDate`, `customerdata.BirthDate`, `customerdata.CustType` FROM `customer`, `customerdata`, `transaction` WHERE `customer.CID` = 'transaction.CID' AND `customerdata.CustDID` = 'transaction.CustDID';
    
    I think I did that right...

    However, this method could be a pain in the ass when it comes to inserting data because of MySQL's lack of Foreign Key support. You'll have to do multiple queries, which I really hate doing. I try to keep as minimal queries on a single page as possible.
     

Share This Page