pietbez Posted February 27, 2008 Share Posted February 27, 2008 as you can probably see from my scripting, i am very new at this. this script is working but somehow i get the feeling that there is a better or neater or a right way to do this script. i would love to learn from the start how to do it right. can anyone iprove on this script so that the outcome is still the same? <? include('../qazwsxedc/config.php'); mysql_connect($server,$username,$password); @mysql_select_db($database) or die ("Unable to connect to the database"); $year=$_POST['year']; $month=$_POST['month']; $min="$year".'.'."$month".'.'.'00'; $max="$year".'.'."$month".'.'.'32'; $do=mysql_query ("SELECT event_date FROM calendar_events WHERE event_date>'$min' and event_date<'$max' and status='confirmed'and event_type='gig' "); $x=mysql_num_rows($do); if ($x>0) { while ($row = mysql_fetch_array($do, MYSQL_ASSOC)) { $dates = explode("-",$row["event_date"]); $name.="$dates[2]".'.'."$dates[1]".'.'."$dates[0]".' '; } ;} echo "resultstr=".$name; ?> Quote Link to comment Share on other sites More sharing options...
btherl Posted February 27, 2008 Share Posted February 27, 2008 I've simplified some bits .. I also renamed some variables to more suitable names, and improved the error checking on the mysql query. As for improving the code in general, I think you'll need to tell us what it does The other change I made is to protect against sql injection by allowing only digits in $year and $month. And the other thing is the syntax of building strings. I think that this way is clearer. The {} is to protect a "complex" variable, like an array with an index, when it's inside a double quoted string. <? include('../qazwsxedc/config.php'); mysql_connect($server,$username,$password); mysql_select_db($database) or die ("Unable to connect to the database"); $year=$_POST['year']; $month=$_POST['month']; # Sanitize to protect against SQL injection $year = preg_replace('|[^[:digit:]]|', '', $year); $month = preg_replace('|[^[:digit:]]|', '', $month); $min="$year.$month.00"; $max="$year.$month.32"; $sql = "SELECT event_date FROM calendar_events WHERE event_date>'$min' and event_date<'$max' and status='confirmed'and event_type='gig' "; $result = mysql_query($sql) or die("Error in $sql\n" . mysql_error()); $x=mysql_num_rows($result); while ($row = mysql_fetch_array($result, MYSQL_ASSOC)) { $dates = explode("-",$row["event_date"]); $name.="{$dates[2]}.{$dates[1]}.{$dates[0]} "; } echo "resultstr=".$name; ?> Quote Link to comment Share on other sites More sharing options...
flamerail Posted February 27, 2008 Share Posted February 27, 2008 Heres a function I wrote to protect myself against sql injection. <? //Removes EVIL for mysql entrys function safe($string){ $string = trim($string); $string = stripslashes($string); $string = mysqli_real_escape_string(core_db(),$string); return $string; } ?> Use it like this > safe($_GET['whatev']) It will work for any variable being posted. Be sure to change the mysql type to whatever your using, I suggest mysqli any way. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.