[SOLVED] clumsy scripting

[pietbez]

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;
?>

[btherl]

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;
?>

[flamerail]

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.