Jump to content


Photo

better way to code this?


  • Please log in to reply
9 replies to this topic

#1 Cell0518

Cell0518
  • Members
  • PipPipPip
  • Advanced Member
  • 36 posts
  • LocationUSA / Japan (US Navy)

Posted 28 September 2006 - 02:23 AM

Hi,
I have a question.  When coding a update form, I want the data to be pulled from a MySQL DB.

In trying to code better and I'm looking to have a better layout for my code, so it seems less cluttered and more stuctured and possibly using a template.

My current code is similar to this...

<?php

$act = $_GET['act'];
if($act == "") {
$query = "GET title FROM table1";
$db->query($query)
while($row = mysqli_fetch_array($result, MYSQL_ASSOC)) 
{ 
echo "<html>\n<head>....Update Data";
echo "<input type=\"text\" value=\"".$row['title']."\">";
echo "<input type=\"submit\" calue=\"\">";
}
}
if($act == "update") {
$query = ".....";
if($db->query($query)) {
$msg = "Successful" }
else { $msg = "Unsuccessful" }
echo "<html>\n<head>....Data Results..";
echo $msg;
echo "</body></html>";
}

?>

Is there a better way to code something like this? (as in, separating the HTML from the results...)

Thanks,
Chris
Chris

#2 warewolfe

warewolfe
  • Members
  • PipPipPip
  • Advanced Member
  • 57 posts
  • LocationOtago, NZ

Posted 28 September 2006 - 03:30 AM

Hej,
I don't know if that is just a typo for the example but

if($act = "") will always be true as it is true you are assigning $act the value of ""

  some people recommend doing an if check

if("" == $act) {do something}

  because if you mistype and print

if("" = $act) {do somthing}

you'll find out pretty quickly.

As for code layout, try bracketing and indenting in a consistent manner. All k&R or all Allman style. Mixing and matching makes for hard reading.

#3 Cell0518

Cell0518
  • Members
  • PipPipPip
  • Advanced Member
  • 36 posts
  • LocationUSA / Japan (US Navy)

Posted 28 September 2006 - 05:34 AM

what is the difference? ( if ($act == "") and  if ("" == $act) )

Also, I don't want to sound stupid, but what is K&R and Allman?

( I updated the original post :) )

Thanks,
Chris
Chris

#4 Daniel0

Daniel0
  • Staff Alumni
  • Advanced Member
  • 11,956 posts

Posted 28 September 2006 - 05:57 AM

Nothing... what is difference between 1+2=3 and 3=1+2? (nothing)

Here is it rewritten:
<?php
if(empty($_GET['act']))
{
	$result = $db->query("GET title FROM table1");
	while($row = mysqli_fetch_array($result, MYSQL_ASSOC)) 
	{
		echo <<<EOF
<html>
<head>....Update Data
<input type="text" value="{$row['title']}" />
<input type="submit" value="">
EOF;
	}
}
else if($_GET['act'] == "update")
{
	$msg = $db->query(".....") ? "Sucessful" : "Unsucessful";
	
	echo <<<EOF
<html>
<head>....Data Results
{$msg}
</body>
</html>
EOF;
}
?>

You might want to consider using a switch instead. And you need to work on your HTML.

#5 Cell0518

Cell0518
  • Members
  • PipPipPip
  • Advanced Member
  • 36 posts
  • LocationUSA / Japan (US Navy)

Posted 28 September 2006 - 06:27 AM

Hi, this was an example, simplified to not take up too much room. :)  How does the <<<EOF .... EOF; work? (I'm taking it that anything within {} is parsed, everything else is not?)

In the possiblity of using a switch, I assume the way to implement the pieces of code more effectively would be to include them from separate the files?

My site would eventually be a news system (with image uploads and a WYSIWIG editor).  If you know of any tutorials that would help, I would be greatly appreciated.

Thanks,
Chris
Chris

#6 Flukey

Flukey
  • Members
  • PipPipPip
  • Advanced Member
  • 39 posts

Posted 28 September 2006 - 09:24 AM

Nothing... what is difference between 1+2=3 and 3=1+2? (nothing)

Here is it rewritten:

<?php
if(empty($_GET['act']))
{
	$result = $db->query("GET title FROM table1");
	while($row = mysqli_fetch_array($result, MYSQL_ASSOC)) 
	{
		echo <<<EOF
<html>
<head>....Update Data
<input type="text" value="{$row['title']}" />
<input type="submit" value="">
EOF;
	}
}
else if($_GET['act'] == "update")
{
	$msg = $db->query(".....") ? "Sucessful" : "Unsucessful";
	
	echo <<<EOF
<html>
<head>....Data Results
{$msg}
</body>
</html>
EOF;
}
?>

You might want to consider using a switch instead. And you need to work on your HTML.


You're forgetting about checking the input for SQL injection.

#7 warewolfe

warewolfe
  • Members
  • PipPipPip
  • Advanced Member
  • 57 posts
  • LocationOtago, NZ

Posted 28 September 2006 - 10:06 AM

Hej,

  The difference between
case 1 ($var =="")  and
case 2 ("" == $var)
is this,

if you forget to add the second = sign in case 2, which happens often as your first unedited post shows, then the program will fail right away and let you know exactly where it fails and is very easy to debug.

  If you forget the second = sign in case 1 then the 'if' test will always return true, and sometimes your program will work and sometimes it won't. Much harder to debug.

  K&R is a style of code presentation (indenting and bracketing), so is Allman. I prefer Allman style as it is easier to match brackets but it doesn't really matter which one you do as long as you do one consistently.

check out http://en.wikipedia....ki/Indent_style


WW.

#8 Daniel0

Daniel0
  • Staff Alumni
  • Advanced Member
  • 11,956 posts

Posted 28 September 2006 - 01:30 PM

You're forgetting about checking the input for SQL injection.


No I didn't... you cant check for SQL injection as there is no variables in the query.

#9 xyn

xyn
  • Members
  • PipPipPip
  • Advanced Member
  • 779 posts
  • LocationNorthampton

Posted 28 September 2006 - 01:38 PM

This should work...
<?php
$act = $_GET['act'];
if( !$act ) {

$query = mysql_query("SELECT title FROM table1");
while($row = mysql_fetch_array($result, MYSQL_NUM)) 

{ 
	echo "<html>\n<head>....Update Data";
	echo "<input type=\"text\" value=\"".$row['title']."\">";
	echo "<input type=\"submit\" calue=\"\">";
}

} elseif($act == "update") {

$query = ".....";
if($db->query($query))
{
	$msg = "Successful";
}
else
{
	$msg = "Unsuccessful";
}
echo "<html>\n<head>....Data Results..";
echo $msg;
echo "</body></html>";
}

?>


#10 Cell0518

Cell0518
  • Members
  • PipPipPip
  • Advanced Member
  • 36 posts
  • LocationUSA / Japan (US Navy)

Posted 28 September 2006 - 01:41 PM

Thank you for the information, especially showing the $msg = $db->query... line.  This explains something I've wondered about for a while.

As far as my site, it would eventually be a news system (with image uploads and a WYSIWIG editor).  I'm looking at a modular design, similar to most forums, ie: example.com/?mod=news&act=update.  How are you able to make a single php file that references all of the data passed by $_GET?  My current page does most of this, but I'm looking to clean the code up....as a lot of the html is sprinkled in and part is just simply echoed.

I'm looking at making each part of the news "CMS" modular, as you can see in the example URL, which is, as I said, similar to most news pages I've seen..

(ie * http://www.theinquir...x?article=34724 * -> when default.aspx sees ?article=, it knows it is an article and it needs to lookup something in (a database?) that has the id of 34724, which is anything after ?article=.)

I know that this specific forum uses semi-colons vs ampersands, but how is it all linked?  What code is helping tell that when action=post, you need to get all of the required data, and give the end-user a text box to write in and when they "submit", it goes into a database?

Also, if anyone has seen any tutorials, etc that may help answer this question, please post them. 

Again,
I know I am asking a lot, but I feel that the best way for me to learn my php is by making something.

Thanks,
Chris
Chris




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users