Back To Programming / Scripting / Coding Forum

Feedback on page coding.

  • digisales
  • Newbie
  • Newbie
  • No Avatar
  • Joined: Apr 21, 2009
  • Posts: 9
  • Status: Offline

Post June 27th, 2009, 12:02 pm

Over the past several months I have been making an effort to learn PHP on my own. I have created some web pages that call data from mysql, and during my testing of them they all seem to be working just fine.

I'm considering actually using these pages on a website but, since I'm new to this, I don't yet have confidence in them. If possible, I'd like someone to look through the coding I have posted below, and give me their feedback. Basically, what I have done is taken an actual page, stripped out all the HTML, and left only the php.

I am interested in any comments about how this page is structured, but I am especially concerned that I don't have something in it that could disrupt my hosting servers. Throughout the page, various bits of data are being called from two different databases, and I don't want to create a problem of any kind - like leaving connections open for example. Thanks for any help.


Code: [ Select ]
<HTML><HEAD><TITLE></TITLE>
</HEAD>
<BODY>
 
<?php
$db = mysql_connect("localhost", "user","pwd");
mysql_select_db("dbase1", $db);
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["f_name"]; echo" ".$myrow["l_name"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["f_name"]; echo" ".$myrow["l_name"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["ref_by"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["co"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["onrf"]; echo" ".$myrow["onrl"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["co"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["onrf"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["onrp"];;
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["onre"];;
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["onrf"];;
?>
 
--- VARIOUS HTML --------
 
<?php
$db = mysql_connect("localhost", "user","pwd");
mysql_select_db("DBASE2", $db);
$id = mysql_real_escape_string($_GET['id']);
$result = mysql_query("SELECT col FROM users WHERE id='X'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["col"];
?>
 
--- VARIOUS HTML --------
 
<?php
$id = mysql_real_escape_string($_GET['id']);
$result = mysql_query("SELECT col FROM users WHERE id='X'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["col"];
?>
 
--- VARIOUS HTML --------
 
<?php
$db = mysql_connect("localhost", "user","pwd");
mysql_select_db("dbase1", $db);
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["f_name"]; echo" ".$myrow["l_name"];
?>
 
--- VARIOUS HTML --------
 
<?php
$a = mysql_real_escape_string($_GET['a']);
$result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
$myrow = mysql_fetch_array($result);
echo $myrow["f_name"]; echo" ".$myrow["l_name"];
?>
 
--- VARIOUS HTML --------
 
</BODY></HTML>
  1. <HTML><HEAD><TITLE></TITLE>
  2. </HEAD>
  3. <BODY>
  4.  
  5. <?php
  6. $db = mysql_connect("localhost", "user","pwd");
  7. mysql_select_db("dbase1", $db);
  8. $a = mysql_real_escape_string($_GET['a']);
  9. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  10. $myrow = mysql_fetch_array($result);
  11. echo $myrow["f_name"]; echo" ".$myrow["l_name"];
  12. ?>
  13.  
  14. --- VARIOUS HTML --------
  15.  
  16. <?php
  17. $a = mysql_real_escape_string($_GET['a']);
  18. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  19. $myrow = mysql_fetch_array($result);
  20. echo $myrow["f_name"]; echo" ".$myrow["l_name"];
  21. ?>
  22.  
  23. --- VARIOUS HTML --------
  24.  
  25. <?php
  26. $a = mysql_real_escape_string($_GET['a']);
  27. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  28. $myrow = mysql_fetch_array($result);
  29. echo $myrow["ref_by"];
  30. ?>
  31.  
  32. --- VARIOUS HTML --------
  33.  
  34. <?php
  35. $a = mysql_real_escape_string($_GET['a']);
  36. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  37. $myrow = mysql_fetch_array($result);
  38. echo $myrow["co"];
  39. ?>
  40.  
  41. --- VARIOUS HTML --------
  42.  
  43. <?php
  44. $a = mysql_real_escape_string($_GET['a']);
  45. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  46. $myrow = mysql_fetch_array($result);
  47. echo $myrow["onrf"]; echo" ".$myrow["onrl"];
  48. ?>
  49.  
  50. --- VARIOUS HTML --------
  51.  
  52. <?php
  53. $a = mysql_real_escape_string($_GET['a']);
  54. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  55. $myrow = mysql_fetch_array($result);
  56. echo $myrow["co"];
  57. ?>
  58.  
  59. --- VARIOUS HTML --------
  60.  
  61. <?php
  62. $a = mysql_real_escape_string($_GET['a']);
  63. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  64. $myrow = mysql_fetch_array($result);
  65. echo $myrow["onrf"];
  66. ?>
  67.  
  68. --- VARIOUS HTML --------
  69.  
  70. <?php
  71. $a = mysql_real_escape_string($_GET['a']);
  72. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  73. $myrow = mysql_fetch_array($result);
  74. echo $myrow["onrp"];;
  75. ?>
  76.  
  77. --- VARIOUS HTML --------
  78.  
  79. <?php
  80. $a = mysql_real_escape_string($_GET['a']);
  81. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  82. $myrow = mysql_fetch_array($result);
  83. echo $myrow["onre"];;
  84. ?>
  85.  
  86. --- VARIOUS HTML --------
  87.  
  88. <?php
  89. $a = mysql_real_escape_string($_GET['a']);
  90. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  91. $myrow = mysql_fetch_array($result);
  92. echo $myrow["onrf"];;
  93. ?>
  94.  
  95. --- VARIOUS HTML --------
  96.  
  97. <?php
  98. $db = mysql_connect("localhost", "user","pwd");
  99. mysql_select_db("DBASE2", $db);
  100. $id = mysql_real_escape_string($_GET['id']);
  101. $result = mysql_query("SELECT col FROM users WHERE id='X'", $db) or die(mysql_error());
  102. $myrow = mysql_fetch_array($result);
  103. echo $myrow["col"];
  104. ?>
  105.  
  106. --- VARIOUS HTML --------
  107.  
  108. <?php
  109. $id = mysql_real_escape_string($_GET['id']);
  110. $result = mysql_query("SELECT col FROM users WHERE id='X'", $db) or die(mysql_error());
  111. $myrow = mysql_fetch_array($result);
  112. echo $myrow["col"];
  113. ?>
  114.  
  115. --- VARIOUS HTML --------
  116.  
  117. <?php
  118. $db = mysql_connect("localhost", "user","pwd");
  119. mysql_select_db("dbase1", $db);
  120. $a = mysql_real_escape_string($_GET['a']);
  121. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  122. $myrow = mysql_fetch_array($result);
  123. echo $myrow["f_name"]; echo" ".$myrow["l_name"];
  124. ?>
  125.  
  126. --- VARIOUS HTML --------
  127.  
  128. <?php
  129. $a = mysql_real_escape_string($_GET['a']);
  130. $result = mysql_query("SELECT * FROM Xref WHERE a='$a'", $db) or die(mysql_error());
  131. $myrow = mysql_fetch_array($result);
  132. echo $myrow["f_name"]; echo" ".$myrow["l_name"];
  133. ?>
  134.  
  135. --- VARIOUS HTML --------
  136.  
  137. </BODY></HTML>
  • Anonymous
  • Bot
  • No Avatar
  • Joined: 25 Feb 2008
  • Posts: ?
  • Loc: Ozzuland
  • Status: Online

Post June 27th, 2009, 12:02 pm

  • Nightslyr
  • Proficient
  • Proficient
  • No Avatar
  • Joined: Sep 21, 2005
  • Posts: 274
  • Status: Offline

Post June 27th, 2009, 3:14 pm

1. In general, your PHP structure is a bit off. While you can freely switch between PHP and HTML in your files, it's generally not a good idea. The reason being that as your pages grow in complexity, it becomes harder to maintain when various technologies are strewn throughout. You should instead process all data first, build up your output, then send out the HTML at the end.

2. Given how many times you execute the same code, you should refactor it. Do you need to continuously take data from the db? Why couldn't you load it all at the beginning, then output it when needed (to, again, echo my first point)? Why two separate db's? Why not different tables in the same db?

3. You should verify that $a is a legit value and gracefully handle it if it's not rather than merely echoing out a mysql error (which are ugly) and quitting the script.
  • Bogey
  • Bogey
  • Genius
  • User avatar
  • Joined: Jul 14, 2005
  • Posts: 8212
  • Loc: USA
  • Status: Offline

Post June 27th, 2009, 7:18 pm

You don't need to repeat the codes like you did (as Nightslyr mentioned). You could do most of the PHP stuff before you even put <html>. The only PHP you would have in the actual HTML part is like loops to echo and format arrays.

Or you could learn the use of list() with explode(). Those could come in handy as well.
"Bring forth therefore fruits meet for repentance:" Matthew 3:8
  • Nightslyr
  • Proficient
  • Proficient
  • No Avatar
  • Joined: Sep 21, 2005
  • Posts: 274
  • Status: Offline

Post June 28th, 2009, 2:03 pm

Bogey wrote:
The only PHP you would have in the actual HTML part is like loops to echo and format arrays.


If done right, you wouldn't even need that. There's really no reason why you couldn't build these structures in the processing phase, store them in variables, and simply echo them at the right point in the document. For things like building tables based off db data, it just seems easier to attempt it before any HTML is sent...that way, if there's an error, you could just echo an empty string (where the built-up output would be) followed by an error message.

I'd just rather do something like:
Code: [ Select ]
<?php
   //assume I've successfully connected to my db
 
   $results = mysql_query("SELECT * FROM my_database");
   $output = "";
   $error = "";
 
   if($results)
   {
      $output .= "<table>";
 
      while($row = mysql_fetch_assoc($results))
      {
         $output .= "<tr><td>{$row['name']}</td><td>{$row['data']}</td></tr>";
      }
 
      $output .= "</table><br />";
   }
   else
   {
      $error .= "Could not fetch results from the database.  Please contact the webmaster if the problem persists.";
   }
?>
 
<!-- elsewhere in the document -->
 
<div id="content">
   <?php echo $output; ?>
   <span id="error"><?php echo $error; ?></span>
</div>
  1. <?php
  2.    //assume I've successfully connected to my db
  3.  
  4.    $results = mysql_query("SELECT * FROM my_database");
  5.    $output = "";
  6.    $error = "";
  7.  
  8.    if($results)
  9.    {
  10.       $output .= "<table>";
  11.  
  12.       while($row = mysql_fetch_assoc($results))
  13.       {
  14.          $output .= "<tr><td>{$row['name']}</td><td>{$row['data']}</td></tr>";
  15.       }
  16.  
  17.       $output .= "</table><br />";
  18.    }
  19.    else
  20.    {
  21.       $error .= "Could not fetch results from the database.  Please contact the webmaster if the problem persists.";
  22.    }
  23. ?>
  24.  
  25. <!-- elsewhere in the document -->
  26.  
  27. <div id="content">
  28.    <?php echo $output; ?>
  29.    <span id="error"><?php echo $error; ?></span>
  30. </div>


Instead of:
Code: [ Select ]
<?php
   $results = mysql_query("SELECT * FROM my_database");
?>
 
<!-- ... -->
 
<div id="content">
   <?php
      if($results)
      {
         echo "<table>";
 
         while($row = mysql_fetch_assoc($results))
         {
            echo "<tr><td>{$row['name']}</td><td>{$row['data']}</td></tr>";
         }
 
         echo "</table><br />";
      }
      else
      {
         echo "<span id=\"error\">Could not fetch results from the database.  Please contact the webmaster if the problem persists.</span>";
      }
   ?>
</div>
  1. <?php
  2.    $results = mysql_query("SELECT * FROM my_database");
  3. ?>
  4.  
  5. <!-- ... -->
  6.  
  7. <div id="content">
  8.    <?php
  9.       if($results)
  10.       {
  11.          echo "<table>";
  12.  
  13.          while($row = mysql_fetch_assoc($results))
  14.          {
  15.             echo "<tr><td>{$row['name']}</td><td>{$row['data']}</td></tr>";
  16.          }
  17.  
  18.          echo "</table><br />";
  19.       }
  20.       else
  21.       {
  22.          echo "<span id=\"error\">Could not fetch results from the database.  Please contact the webmaster if the problem persists.</span>";
  23.       }
  24.    ?>
  25. </div>


Overall, IMO, it just looks cleaner, and is easier to maintain, if the processing is all done before the output is sent.
  • Bogey
  • Bogey
  • Genius
  • User avatar
  • Joined: Jul 14, 2005
  • Posts: 8212
  • Loc: USA
  • Status: Offline

Post June 28th, 2009, 8:50 pm

Didn't think of that...
"Bring forth therefore fruits meet for repentance:" Matthew 3:8
  • Rabid Dog
  • Web Master
  • Web Master
  • User avatar
  • Joined: May 21, 2004
  • Posts: 3229
  • Loc: South Africa
  • Status: Offline

Post June 29th, 2009, 5:14 pm

Ok here I go again :)

Having HTML embedded in code is just a really bad idea. Rather look at implementing a templating mechanism and have your code fill in the blanks if you will.

For things like grid views what you might want to do is define static Strings inside some sort of 'tag' library. that way you can call the static string with a friendly variable name and format it using str_replace or the likes.

Let me know if I should go more indepth with this, hey, prehaps a tutorial is in order!
Watch me grow
  • SpooF
  • ٩๏̯͡๏۶
  • Bronze Member
  • User avatar
  • Joined: May 22, 2004
  • Posts: 3415
  • Loc: Richland, WA
  • Status: Offline

Post June 29th, 2009, 6:59 pm

Rabid Dog, do you do much of any PHP work anymore? I'm kind of curious to hear what you have to say about some of the current popular frameworks such as the Zend, Cake and CodeIgniter frameworks in reference to the MVC model and templating.
#define NULL (::rand() % 2)
  • Nightslyr
  • Proficient
  • Proficient
  • No Avatar
  • Joined: Sep 21, 2005
  • Posts: 274
  • Status: Offline

Post June 30th, 2009, 4:43 am

Rabid Dog wrote:
Ok here I go again :)

Having HTML embedded in code is just a really bad idea. Rather look at implementing a templating mechanism and have your code fill in the blanks if you will.

For things like grid views what you might want to do is define static Strings inside some sort of 'tag' library. that way you can call the static string with a friendly variable name and format it using str_replace or the likes.

Let me know if I should go more indepth with this, hey, prehaps a tutorial is in order!


FWIW, I think a templating tutorial is a great idea.
  • UPSGuy
  • Lurker ಠ_ಠ
  • Web Master
  • User avatar
  • Joined: Jul 25, 2005
  • Posts: 2735
  • Loc: Nashville, TN
  • Status: Offline

Post June 30th, 2009, 6:21 am

Quote:
hey, prehaps a tutorial is in order!


He's not just back ladies and gents, he's back with a vengeance :) WB Rabid Dog. I'm happy to see you're bringing your knowledge back to the prog forum.
I'd love to change the world, but they won't give me the source code.
  • Rabid Dog
  • Web Master
  • Web Master
  • User avatar
  • Joined: May 21, 2004
  • Posts: 3229
  • Loc: South Africa
  • Status: Offline

Post June 30th, 2009, 12:28 pm

I know this ain't the place to do it but I am gonna do it anyways :)

Thanks all for the warm welcome back and I look forward to being productive on the forum again, my apologies for being away so long and I have a stack of new things to share HOOAH!
Watch me grow
  • Rabid Dog
  • Web Master
  • Web Master
  • User avatar
  • Joined: May 21, 2004
  • Posts: 3229
  • Loc: South Africa
  • Status: Offline

Post June 30th, 2009, 12:32 pm

SpooF wrote:
Rabid Dog, do you do much of any PHP work anymore? I'm kind of curious to hear what you have to say about some of the current popular frameworks such as the Zend, Cake and CodeIgniter frameworks in reference to the MVC model and templating.



Unfortunately Spoof I am the guy that recons I can do it better than everyone else :) But my many focus over the past three years has been enterprise application integration using mainly Java, C# and SOAP.

I just find PHP to not be an enterprise solution as it is difficult to find 2am code errors that a compiler picks up for you. That being said I would be interested to have a fiddle with them and render my feed back :)
Watch me grow

Page 1 of 1

Post Information

  • Total Posts in this topic: 11 posts
  • Users browsing this forum: No registered users and 125 guests
  • You cannot post new topics in this forum
  • You cannot reply to topics in this forum
  • You cannot edit your posts in this forum
  • You cannot delete your posts in this forum
  • You cannot post attachments in this forum
 
cron
 

© 2011 Unmelted, LLC. Ozzu® is a registered trademark of Unmelted, LLC.