4
\$\begingroup\$

I am looking for a suggestion or advises about some code I have written. I build an online application about 3 month ago the app registers new buildings including description names and so on. The app works perfectly and does what it suppose to. While working on it I was constantly learning and improving my skills. I did include the internal CSS as it was designed specifically for this page but I also used external one as well. I used a lot of ajax, php, and js. I looked at many tutorial and articles while working on this. My main question is
1) some suggestion toward improving the structure

2) is the code similar to something that in a real world in real companies programmer write. 3) Is it good for someone who just started it career and the first major web app he is written. Here is a sample file again it all working I do not want you to go thru every line just suggestions. 

<!DOCTYPE html>
 <style type="text/css">
 /* demo styles */
 fieldset { border:0; }
 select {
 width: 400px;
 }
 a.ggg{
 color: #003366;
 font-size: x-large;
 font-family: Monotype Corsiva;
 }
 a.gg{
 color: #003366;
 font-size: medium;
 font-family: Monotype Corsiva;
 }
 .darkbg{
 background:#ddd !important;
 }
 #status{ font-family:Arial; padding:5px; }
 ul#files{ list-style:none; padding:0; margin:0; }
 ul#files li{ margin-bottom:2px; width:200px; float:left; }
 ul#files li img{ max-width:180px; max-height:150px; }
 .success{ background:#FFCC99; border:1px solid #FF9933; }
 .error{ background:#f0c6c3; border:1px solid #cc6622; }
 #button { padding: .5em 1em; text-decoration: none; }
 #effect { width: 700px; height: 200px; padding: 0.4em; position: relative; }
 #effect h3 { margin: 0; padding: 0.4em; text-align: center; }
 </style>
<html>
<head>
 <meta http-equiv="content-type" content="text/html; charset=utf-8" />
 <title></title>
 <link href="styles.css" rel="stylesheet" type="text/css" media="screen" />
 <link type="text/css" href="css/custom-theme/jquery-ui-1.8.5.custom.css" rel="stylesheet" />
 <script type="text/javascript" src="js/ajaxupload.3.5.js" ></script>
 <script type="text/javascript" src="js/jquery-1.4.2.min.js"></script>
 <script type="text/javascript" src="js/jquery-ui-1.8.5.custom.min.js"></script>
 <script type="text/javascript" src="tags.js"></script>
 <script type="text/javascript" src="ui/jquery.ui.core.js"></script>
 <script type="text/javascript" src="ui/jquery.ui.widget.js"></script>
 <script type="text/javascript" src="ui/jquery.ui.position.js"></script>
 <script type="text/javascript" src="ui/jquery.ui.selectmenu.js"></script>
 <script type="text/javascript">
 $(function(){
 // gui function for builodin select box gui
 $('select#Building').selectmenu({
 style:'popup',
 maxHeight: 150
 });
 $('select#Building4').selectmenu({
 style:'popup',
 maxHeight: 150
 });
 $('select#Building24').selectmenu({
 style:'popup',
 maxHeight: 150
 });
 });
 //a custom format option callback
 // builds gui for tabs
 $(function() {
 $( "#tabs" ).tabs();
 });
 // gui function for buttons 
 $(function() {
 $( "#search" ).button();
 $( "#delete" ).button();
 $( "#view" ).button();
 $( "#insert" ).button();
 $( "#upload" ).button();
 $( "#finish" ).button();
 $( "#thub" ).button();
 $( "#insroom" ).button();
 $( "#clearfields" ).button();
 $("#date").datepicker();
 });
 // ajax Function used to retrive all question runs getallquestions.php
 function allQuestions(){
 $.ajax({
 type: "POST",
 url: "getallquestions.php",
 success: function(html){
 $("#allquestions").html(html);
 }
 });
 }
 // ajax function send date to the getquestionsbydate.php and brings questions 
 function Questionsbydate(){
 $.ajax({
 type: "POST",
 url: "getquestionsbydate.php",
 data: "date=" + document.getElementById('date').value,
 success: function(html){
 $("#allquestions").html(html);
 }
 });
 }
 function deletebuilding(){
 // gets building information 
 var x=document.getElementById("Building");
 var building=x.options[x.selectedIndex].text;
 // runs ajax function to delete the building
 $.ajax({
 type: "POST",
 url: "deletebuilding.php",
 data: "building=" + building,
 success: function(html){
 $("#deletebuilding").html(html);
 }
 });
 }
 // gets the building information 
 function update(){
 var x=document.getElementById("Building4");
 var building4=x.options[x.selectedIndex].text;
 // ajax function to run the updatebuilding.php
 $.ajax({
 type: "POST",
 url: "updatebuilding.php",
 data: "building4=" + building4,
 success: function(html){
 $("#updateinfo").html(html);
 }
 });
 }
 function insert(){
 // ajax function to run insertbuilding.php
 $.ajax({
 type: "POST",
 url: "insertbuilding.php",
 data: "building2=" + document.getElementById('building2').value +
 "&list2=" + document.getElementById('list2').value+
 "&NoonMap2=" + document.getElementById('NoonMap2').value+
 "&Description2=" + document.getElementById('Description2').value+
 "&bldg2=" + document.getElementById('bldg2').value,
 success: function(html){
 $("#buli").html(html);
 }
 });
 }
 function EMPTY(){
 // retrives the building information 
 document.getElementById('building2').value ="" ;
 document.getElementById('list2').value="" ;
 document.getElementById('NoonMap2').value="" ;
 document.getElementById('Description2').value="" ;
 document.getElementById('bldg2').value="" ;
 document.getElementById('buli').innerHTML="";
 }
 // runs funvtion to inset the thumbnail to the database
 function thub(){
 $.ajax({
 type: "POST",
 url: "blobpart11.php",
 data: "building2=" + document.getElementById('building2').value,
 success: function(html){
 $("#thumres").html(html);
 }
 });
 }
 // function to run the insert question runs addquestion.php
 function addquestion(){
 $.ajax({
 type: "POST",
 url: "addquestion.php",
 data: "quest=" + document.getElementById('quest').value +
 "&ans="+document.getElementById('ans').value,
 success: function(html){
 $("#allquestions").html(html);
 }
 });
 }
 // function for creating button gui and dialog effect 
 $(function() {
 $( "#submit" ).button();
 function runEffect() {
 var selectedEffect = $( "#effectTypes" ).val();
 var options = {};
 $( "#effect" ).show( selectedEffect, options, 500, callback );
 };
 function callback() {
 };
 // set effect from select menu value
 $( "#button" ).click(function() {
 runEffect();
 return false;
 });
 $( "#effect" ).hide();
 });
 function callback45() {
 $( "#effect:visible" ).removeAttr( "style" ).fadeOut();
 };
 function insertroom(){
 // retrives building information 
 var x=document.getElementById("Building24");
 var building24=x.options[x.selectedIndex].text;
 $.ajax({
 // ajax function used to run the addroom specification.php
 type: "POST",
 url: "addroomspecification.php",
 data: "building24=" + $( "#Building24" ).val() +
 "&Rm_Nm=" + document.getElementById('rm').value+
 "&Filled=" + document.getElementById('fill').value+
 "&Gender=" + document.getElementById('gen').value+
 "&Dimensions=" + document.getElementById('dim').value+
 "&Comments=" + document.getElementById('com').value+
 "&prcd=" + document.getElementById('prcd').value+
 "&bl=" + document.getElementById('bl').value+
 "&rest=" + document.getElementById('rest').value+
 "&Capacity=" + document.getElementById('cap').value,
 success: function(html){
 $("#room").html(html);
 }
 });
 }
 function clearfilds(){
 // function for clearing fields
 document.getElementById('rm').value= "" ;
 document.getElementById('fill').value="" ;
 document.getElementById('gen').value= "" ;
 document.getElementById('dim').value= "" ;
 document.getElementById('com').value= "" ;
 document.getElementById('prcd').value= "" ;
 document.getElementById('bl').value= "" ;
 document.getElementById('rest').value= "" ;
 document.getElementById('cap').value= "" ;
 }
 </script>
</head>
<body>
 <div id="content">
 <!-- header begins -->
 <div id="menu">
 <ul>
 <li id="button1"><a href="#" title="" >Home</a></li>
 <li id="button2"><a href="#" title="">Blog</a></li>
 <li id="button3"><a href="#" title="">Gallery</a></li>
 <li id="button4"><a href="#" title="">About</a></li>
 </ul>
 </div>
 <div id="logo">
 <img src="images/logo.jpg" width="300" height="180" alt="" />
 </div>
 <br /><br />
 <div id="tabs" style="height: 700px; overflow: scroll">
 <ul>
 <li><a href="#tabs-1">Search By Key word</a></li>
 <li><a href="#tabs-2">Search By Specification</a></li>
 <li><a href="#tabs-3">Modify Building Information</a></li>
 <li><a href="#tabs-4">Modify Building Information</a></li>
 <li><a href="#tabs-5">Modifyn</a></li>
 </ul>
 <div id="tabs-1" >
 <br /><br />
 <input id="search" type="button" name="Search by date" value="Search by Date" onclick="Questionsbydate()"/>&nbsp;&nbsp;
 <input id="date" name="date" />&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <input id="view" type="button" name="View All QUESTIONS" value="View All QUESTIONS" onclick="allQuestions()" />
 <br /><br />
 <div id="allquestions"></div>
 <br /><br /><br />
 <div class="demo">
 <div align="center" class="toggler">
 <div style="width: 700px; height: 200px" id="effect" class="ui-widget-content ui-corner-all">
 <h3 class="ui-widget-header ui-corner-all">Add QUESTION</h3>
 <p>
 <a class='gg' >Please Enter The Question</a>&nbsp;&nbsp;<input id="quest" style="width: 300px" name="quest" /><br /><br />
 <a class='gg' >Please Enter The Answer</a>&nbsp;&nbsp;&nbsp;&nbsp;<input id="ans" style="width: 300px" name="ans" />
 </p>
 </div>
 </div>
 <select style="visibility: hidden" name="effects" id="effectTypes">
 <option value="blind">Blind</option>
 </select>
 <center><br /><br /><br />
 <a href="#" id="button" class="ui-state-default ui-corner-all">Add Question</a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <input id="submit" type="button" name="Submit" value="Submit" onclick=" addquestion(), callback45() " /></center>
 </div>
 <div style="display: none" id="dialog" title="Basic dialog">
 <table>
 <tr>
 <td><a>E-Mail</a></td>
 <td><input id="email" name="email" /></td>
 </tr>
 <tr>
 <td><a>Question Asked</a></td>
 <td><textarea id="qu" name="quest" rows="3"></textarea><br /></td>
 </tr>
 <tr>
 <td><a>Question Answer</a></td>
 <td><textarea id="answ" name="quest" rows="3"></textarea><br /></td>
 </tr>
 </table>
 <input id="op" type="button" name="reply" value="Reply" onclick="se()" />
 </div>
 <div id="response"></div>
 </div> <!-- /#tabs-1 -->
 <div id="tabs-2">
 <?php
 $con = mysql_connect("localhost", "root");
 if (!$con) {
 die('Could not connect: ' . mysql_error());
 }
 // retrives the list of buildings 
 mysql_select_db("buildings", $con);
 $result6 = mysql_query("SELECT `Building Name`
 FROM `buildingsinfo`
 WHERE `Building Name` <> ''
 GROUP BY `Building Name` ");
 echo "<center><a class='ggg' >Please select House</a><br /><br /><br /><select name=Building id='Building' value=''>Building Name</option>";
 // printing the list box select command
 while($nt6=mysql_fetch_array($result6)) {
 //Array or records stored in $nt
 $hello=$nt6['Building Name'];
 echo "<option value=$hello>$hello</option>";
 /* Option values are added by looping through the array */
 }
 echo "</select><br /><br /><br />";// Closing of list box
 ?>
 <center>
 <input type="button" id="delete" style="width: 200px; height: 100px; font-size: large" value="DELETE BUILDING" onclick="deletebuilding()">
 </center><br /><br />
 <div id="deletebuilding"></div>
 </div> <!-- /#tabs-2 -->
 <div id="tabs-3">
 <?php
 $con = mysql_connect("localhost", "root");
 if (!$con){
 die('Could not connect: ' . mysql_error());
 }
 // retrives the list of buildings and builds dropdown box
 mysql_select_db("buildings", $con);
 $result6 = mysql_query("SELECT `Building Name`
 FROM `buildingsinfo`
 WHERE `Building Name` <> ''
 GROUP BY `Building Name` ");
 echo "<a class='ggg' >Please select House</a><br /><br /><br /><select name=Building id='Building4' value='' onchange='update()' >Building Name</option>";
 // printing the list box select command
 echo "<option value=ANY>ANY</option>";
 while($nt6=mysql_fetch_array($result6)) {
 //Array or records stored in $nt
 $hello=$nt6['Building Name'];
 echo "<option value=$hello>$hello</option>";
 /* Option values are added by looping through the array */
 }
 echo "</select><br /><br /><br />"; // Closing of list box
 ?>
 <div id="updateinfo"></div>
 </div> <!-- /#tabs-3 -->
 <div id="tabs-4">
 <h2>Step 1 Insert The Building Info</h2>
 <a class='gg' >Building Name</a>&nbsp;&nbsp;<input id="building2" name="building" value="" />
 <br /><br />
 <a class='gg' > List No. </a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <input id="list2" name="list" value="" />
 <br /><br />
 <a class='gg' > No. on Map.</a>&nbsp;&nbsp;&nbsp;&nbsp;
 <input id="NoonMap2" name="NoonMap2" value="" />
 <br /><br />
 <a class='gg' >Description</a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <textarea rows="7" style="width: 200px" id="Description2" name="Description" > </textarea>
 <br /> <br />
 <a class='gg' >Building Initials</a>&nbsp;&nbsp;
 <input id="bldg2" name="Bldg" value="" />
 <br /><br />
 <input id="insert" type="button" name="INSERT" value="insert" onclick=" insert()" />
 <br /><br /><br />
 <h2>Step 2 Insert Insert Thubnail</h2>
 <input id="thub" type="button" name="thub" value="THUBNAIL" onclick="thub()" />
 <br /><br />
 <div id="thumres"></div>
 <br /><br />
 <h2>Step 3 Insert Pictures</h2>
 <div id="buli"></div>
 <br /><br /><br /><br />
 <input id="finish" type="button" name="FINISH" value="FINISH" onclick=" EMPTY()" />
 </div> <!-- /#tabs-4 -->
 <div id="tabs-5">
 <?php
 $con = mysql_connect("localhost", "root");
 if (!$con) {
 die('Could not connect: ' . mysql_error());
 }
 // Retrives a list of buildings and builds dropdown box 
 mysql_select_db("buildings", $con);
 $result6 = mysql_query("SELECT `Building Name` , `Bldg`
 FROM `buildingsinfo`
 WHERE `Bldg` <> ''");
 echo "<a class='ggg' >Please select House</a><br /><br /><br /><select name=Building24 id='Building24' value='' >Building Name</option>";
 // printing the list box select command
 while($nt6=mysql_fetch_array($result6)) {
 //Array or records stored in $nt
 $hello=$nt6['Building Name'];
 echo "<option value=$hello>$hello</option>";
 /* Option values are added by looping through the array */
 }
 echo "</select><br /><br /><br />";// Closing of list box
 ?>
 <a class='gg' >Please Enter Room Number</a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <input id="rm" name="rm" />
 <br /><br />
 <a class='gg' >Please Enter Building Letter</a>
 <input id="bl" name="bl" />
 <br /><br />
 <a class='gg' >Filled</a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <input id="fill" name="fill" />
 <br /><br />
 <a class='gg' >Please Enter Pricing Code</a>&nbsp;&nbsp;&nbsp;
 <input id="prcd" name="prcd" />
 <br /><br />
 <a class='gg' >Please Enter Capacity</a>&nbsp;&nbsp;&nbsp;&nbsp;
 <input id="cap" name="cap" />
 <br /><br />
 <a class='gg' >Please Enter Gender</a>&nbsp;&nbsp;&nbsp;
 <input id="gen" name="gen" />
 <br /><br />
 <a class='gg' >Please Enter Dimension</a>&nbsp;&nbsp;
 <input id="dim" name="dim" />
 <br /><br />
 <a class='gg' >Please Enter Restriction Name</a>
 <input id="rest" name="rest" />
 <br /><br />
 <a class='gg' >Please Enter Comments</a> &nbsp;&nbsp;&nbsp;
 <input id="com" name="com" />
 <br /><br />
 <input id="insroom" type="button" name="insroom" value="Insert Room" onclick="insertroom()" />&nbsp;&nbsp;&nbsp;
 <input id="clearfields" type="button" name="clearfilds" value="Clear Fields/Insert Another Room" onclick="clearfilds()"/>
 <br /><br /><br />
 <div id="room"></div>
 </div> <!-- /#tabs-5 -->
 </div> <!-- /#tabs -->
 </div> <!-- /#content -->
<!-- End demo -->
 <div>
 <br /><br /><br />
 <img src="images/logo_lsu_dsa.JPG" width="231" height="83" alt="" />
 </div>
 <!--footer begins -->
 <div id="footer">
 <p>Copyright 2009. <a href="#">Privacy Policy</a> | <a href="#">Terms of Use</a> | <a href="http://validator.w3.org/check/referer" title="This page validates as XHTML 1.0 Transitional"><abbr title="eXtensible HyperText Markup Language">XHTML</abbr></a> | <a href="http://jigsaw.w3.org/css-validator/check/referer" title="This page validates as CSS"><abbr title="Cascading Style Sheets">CSS</abbr></a></p>
 <p>Design by <a href="http://www.metamorphozis.com/" title="Flash Website Templates">Flash Website Templates</a></p>
 </div><!-- footer ends-->
 <div style="text-align: center; font-size: 0.75em;">
 Design downloaded from <a href="http://www.freewebtemplates.com/">free website templates</a>.
 </div>
</body>
</html>
Jonny Sooter
1,38512 silver badges33 bronze badges
asked Apr 8, 2011 at 3:11
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Welcome to CodeReview.SE. Please post your code in the post and remove the link to the pastebin. All of this is mentioned in the FAQ: codereview.stackexchange.com/faq. Thanks. \$\endgroup\$ Commented Apr 8, 2011 at 11:49
  • \$\begingroup\$ Thank you all!! I was trying to post the code but i guess it to long and not getting displayed properly. \$\endgroup\$ Commented Apr 8, 2011 at 19:35

2 Answers 2

3
\$\begingroup\$

When it comes down to improving the structure there's reallly 2 ways to go about it when it comes to the client side.

I can see that your loading your CSS Directly within the page, this can be a pro when it comes to loading speeds, but with today's internet speeds that's practically nothing.

I would separate each entity of the site, entities being:

  • CSS
  • Javascript

And i would use link / script tag's to include this to your page, the reason for this is that if you have multiple pages you would have to copy and paste each block of a css and javascript to that page, and then when you make a small change you would have edit lots of files.

When it comes to PHP, you seem to be doing the same thing, directly typing the code into the main page, as stated before this would cause issues with updates, so place it within a template and include it:

You can make your templates a lot more flexible this way, here's how i think your layout should look:

<!DOCTYPE html>
<html>
 <?php require('templates/head.php'); ?>
 <body>
 <div class="wrapper">
 <?php require('templates/header.php'); ?>
 <?php require('templates/menu.php'); ?>
 <div id="container">
 <div id="main">
 <?php require('templates/content/main.php'); ?>
 </div>
 <div id="main">
 <?php require('templates/content/sidebar.php'); ?>
 </div>
 </div>
 </div>
 <?php require('templates/content/footer'); ?>
 </body>
</html>

Also another major issue that I see is that your code is extremely invalid, you should google W3C Validator and validate your html

answered Apr 8, 2011 at 9:04
\$\endgroup\$
1
  • 1
    \$\begingroup\$ id="main" twice? \$\endgroup\$ Commented Apr 13, 2011 at 7:42
2
\$\begingroup\$

Your code shows real promise!

Web based systems are complex and you have been able to utilise different elements to achieve your goals: CSS, javascript, PHP, SQL. You are seeking advice, which means you will learn more quickly than most programmers. You have correctly comprehended the basic tools for organising your code, and have also begun good habits like using comments to make your code more readable. All really great to see in a new programmer!

The previous response was excellent on the point of organising your code.

Because you show promise and are asking questions, I will also take the time to comment on some other areas you may improve:

  • Inconsistent indentation
  • Comments could be improved (always the case, even for the most experienced of us!)
  • Inconsistent capitalisation in variable, function and SQL column names (lowercase_with_underscores() is my preference as it saves typing and isMoreReadableThanThisCrapWhichIsCalledCamelCase(), however many OOP-focused coders use the second style. That doesn't make them right, though!)
  • Inconsistent naming of SQL column names. There are various approaches to this, personally I almost always have a primary key column called id, name my tables with a plural (eg: buildings) and so wind up with very readable queries like select id from buildings. It is almost always a bad idea to use CamelCase or Multi Word column names, since they mean that you have to write escaping backquotes around them all the time.
  • Strange SQL queries. Your query SELECT ``Building Name`` , ``Bldg`` FROM ``buildingsinfo`` WHERE ``Bldg`` <> '' blah is asking for apparently every row in the database table. The easier way to do this is simply SELECT column,list,here FROM buildingsinfo. There is no need for the WHERE clause.
answered Apr 12, 2011 at 2:06
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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