[PHP] Would this work?

[Luke]

Ok, i'ev got this code, and what happens is when you click the download link it wuold go to count.php?id=2 for example.


<?php
include ('config.php');

$id = $_GET['id'];
if (!is_numeric($id))
{
exit;
}
mysql_query ("UPDATE downloads SET total = total + 1 WHERE id = '$id'");
$result = mysql_query("SELECT * FROM downloads WHERE id = '$id'");
$row = mysql_fetch_object ($result);
header("Location: " . $row->url);
?>Would that add a "number times downloaded" to the SQL database and then forward them to the download?

Thanks
Luke

Moved by Invent (Forum Moderator) from Designing & Development: Please post in the correct forum next time, thanks :).

[Source]

<?php
include ('config.php');

//Basic Filter for ID (Stops SQL Injections)
$id = mysql_real_escape_string(htmlentities($_GET['id'], ENT_QUOTES));

//Would work but the filter ^ is there just in case
if (!is_numeric($id))
{
exit;
}

//Update the download count, I do this a longer but easier to edit way
//Run query
$query = mysql_query( "SELECT * FROM `downloads` WHERE `id` = '$id'" );

//Grab the data into an array (can use object if you want)
$data = mysql_fetch_array( $total );

//Add 1 to the total, I did it this long way as here you can now do more complex
//Sums which I have had todo in the past
$totalnew = $data['total'] + 1;

//Update total to the one we just calculated
$update = mysql_query ("UPDATE `downloads` SET `total` = $totalnew WHERE `id` = '$id'");

//Run the query (added ` for more compatibilty)
$result = mysql_query("SELECT * FROM `downloads` WHERE `id` = '$id'");

//Get the data into a usable object
$row = mysql_fetch_object ($result);

//Head off to the URL from the database
header("Location: " . $row->url);

?>
You did some things I personally havn't seen before, and they probably were correct. I just tidied it up a bit and did the download count slightly different as I find it easier to work with multiple lines (gives extra compatibility when you want todo more complex sums in the future).

More experienced php programmers will probably tell me how bad that is, sorry if it is - feel free to correct me.

[Protege]

<?php
include( 'config.php' );

function cleanMe( $yay )
{
$str = mysql_real_escape_string( $yay );
return( strip_tags( $str ) );
// add some more cleaning if you want lalala..
}

/*
You dont really need the function as you only call it once
( well only need to, as you could just clean $id )
Basically what source said, use it if you understand it.
*/

$id = $_GET[ 'id' ]; // i'll keep that

if( is_numeric( $id ) )
{
if( mysql_query( 'UPDATE `downloads` SET `total` = `total` + 1 WHERE `id` = "' . cleanMe( $id ) . '"' ) )
{
if( $mysqlQuery = mysql_query( 'SELECT * FROM `downloads` WHERE `id` = "' . cleanMe( $id ) . '"' ) )
{
if( mysql_num_rows( $mysqlQuery ) >= 1 )
{
if( $mysqlFetch = mysql_fetch_array( $mysqlQuery ) )
{
// wayy
headers( 'Location: ' . $mysqlFetch[ 'url' ] );
}
else
{
echo( 'Error sxi no rows' );
}
}
else
{
echo( 'Omg no rowzz' );
}
}
else
{
echo( 'Like no accezz or errorz in dbazes' );
}
}
}
else
{
// try not to use exit tbh its nasty (unless it rly satisfies you)
echo( 'Booby error' );
}
?>
Yours will work but theres some changes id make, and you probs get someone like jewball coming and putting something useless in.


Ok, i'ev got this code, and what happens is when you click the download link it wuold go to count.php?id=2 for example.


<?php
include ('config.php');

$id = $_GET['id'];
if (!is_numeric($id))
{
exit;
}
mysql_query ("UPDATE downloads SET total = total + 1 WHERE id = '$id'");
$result = mysql_query("SELECT * FROM downloads WHERE id = '$id'");
$row = mysql_fetch_object ($result);
header("Location: " . $row->url);
?>Would that add a "number times downloaded" to the SQL database and then forward them to the download?

Thanks
Luke

[Source]

Protege's is more complex and if you understand it, use it, but make sure you actually read through and see what he's doing so you can improve aswel. I get fed up with the countless amount of times I see people just copy and use code without knowing what it does.

[Hypertext]

<?php
include( 'config.php' );

$id = $_GET[ 'id' ]; // i'll keep that

if( is_numeric( $id ) )
{
if( mysql_query( "UPDATE `downloads` SET `total` = `total` + 1 WHERE `id` = '$id'" ) )
{
if( $mysqlQuery = mysql_query( "SELECT * FROM `downloads` WHERE `id` = '$id'" ) )
{
if( mysql_num_rows( $mysqlQuery ) > 0 )
{
if( $mysqlFetch = mysql_fetch_array( $mysqlQuery ) )
{
// wayy
header( 'Location: ' . $mysqlFetch[ 'url' ] );
}
else
{
echo 'Error sxi no rows';
}
}
else
{
echo 'Omg no rowzz';
}
}
else
{
echo 'Like no accezz or errorz in dbazes';
}
}
}
else
{
// try not to use exit tbh its nasty (unless it rly satisfies you)
echo 'Booby error';
}
?>Just changed a few things, like that function is pointless as you verify that $_GET['id'] is numeric.

I'd suggest making a db wrapper, and instead of all those if's just put it in they're functions. I'd change it to or die(mysql_error()) but dont want to right now. :)

Oh and I changed echo's to not use parenthesis, again, and corrected the spelling of header().

@Protege, remember about language constructs that they don't need parenthesis, and it is generally considered bad practice.. well thats what a guy on phpfreaks forums told me..



@update: fixed quotes

[Invent]

<?php
include( 'config.php' );

$id = $_GET[ 'id' ]; // i'll keep that

if( is_numeric( $id ) )
{
if( mysql_query( "UPDATE `downloads` SET `total` = `total` + 1 WHERE `id` = '". $id ."'" ) )
{
if( $mysqlQuery = mysql_query( "SELECT * FROM `downloads` WHERE `id` = '". $id ."'" ) )
{
if( mysql_num_rows( $mysqlQuery ) > 0 )
{
if( $mysqlFetch = mysql_fetch_array( $mysqlQuery ) )
{
// wayy
header( 'Location: ' . $mysqlFetch[ 'url' ] );
}
else
{
echo( 'Error sxi no rows' );
}
}
else
{
echo( 'Omg no rowzz' );
}
}
else
{
echo( 'Like no accezz or errorz in dbazes' );
}
}
}
else
{
// try not to use exit tbh its nasty (unless it rly satisfies you)
echo 'Booby error';
}
?>
Fixed your errors :)



@Protege, remember about language constructs that they don't need parenthesis, and it is generally considered bad practice.. well thats what a guy on phpfreaks forums told me..


I'm not sure why it would be considered bad practice unless they slow down scripts by a decent amount - which I dont think they do.

[Hypertext]

I'd imagine it would slow scripts down... wouldn't it have to alot memory for it?

@off-topic: loops going downwards are 90% faster than upwards.. i heard it then my friend did benchmarks.. and he changed a 7k line script to that, sped it up immensely.

[Invent]

Which did you fix?
The quotes one? I just fixed them, ty.Fantastic.


Why are you promoting echo('this');?Nothing is wrong with using parenthesis' with language constructs in my opinion. I may run a speed test later to actually see how badly they actually do slow down scripts.

[Source]

It doesn't adversly affect the performance of a tiny snipper of code like that, not even over a big script. Charlie just be simple, you always trip yourself up trying to act smart.

[Hypertext]

Yeh, but Protege always uses them. I'm only trying to explain that it's bad practice. :/

How the **** am I tripping myself up? I'm not even trying to act smart.

@edit I'll benchmark this afternoon (central time)

[Source]

You honestly think I / we care about benchmarks over a page of php. Charlie just don't even bother posting in this thread / anywhere on this forum as you clearly think you are better than most of the people here.

It's the same reason you made unboxing videos for your apple gear, you always have to be one tier ahead of anyone else - your life revolves around you weltering in been better.

[Excellent1]

So hypergeek just because he used brackets to make the code look nice and clean you think it is bad practice. It may slow a large script down a tad, maybe a few msi.. but not that much. If you still think this is the 'wrong' way to go I pity your life.

[Protege]

<?php
include( 'config.php' );

$id = $_GET[ 'id' ]; // i'll keep that

if( is_numeric( $id ) )
{
if( mysql_query( "UPDATE `downloads` SET `total` = `total` + 1 WHERE `id` = '$id'" ) )
{
if( $mysqlQuery = mysql_query( "SELECT * FROM `downloads` WHERE `id` = '$id'" ) )
{
if( mysql_num_rows( $mysqlQuery ) > 0 )
{
if( $mysqlFetch = mysql_fetch_array( $mysqlQuery ) )
{
// wayy
header( 'Location: ' . $mysqlFetch[ 'url' ] );
}
else
{
echo 'Error sxi no rows';
}
}
else
{
echo 'Omg no rowzz';
}
}
else
{
echo 'Like no accezz or errorz in dbazes';
}
}
}
else
{
// try not to use exit tbh its nasty (unless it rly satisfies you)
echo 'Booby error';
}
?>Just changed a few things, like that function is pointless as you verify that $_GET['id'] is numeric.

I'd suggest making a db wrapper, and instead of all those if's just put it in they're functions. I'd change it to or die(mysql_error()) but dont want to right now. :)

Oh and I changed echo's to not use parenthesis, again, and corrected the spelling of header().

@Protege, remember about language constructs that they don't need parenthesis, and it is generally considered bad practice.. well thats what a guy on phpfreaks forums told me..



@update: fixed quotes

I dont see why you even try to make something better cause in all honesty, put mine against a MS timer without the function and I bet, it will be faster. ( not that I actually give a **** about a few ms's )

OH MY GOD HE USES BRAKCETS OMG SUE HIM IM A CEO SUEE SUEE SUEE

if( $noobAlert ) // BRACKETS OMG
{
findNoob( 'hypertext' ); // brackets..
noobBin( 'hypertext' ); // brackets again! :O!
echo 'Noob'; // <-- *** that looks out of place to me oh wait, it looks like you, out of ******* place.
}

if's just put it in they're functions

Does that make sense to you guys? I swear if it does im ******ed or something.

if( mysql_num_rows( $mysqlQuery ) > 0 )

What was wrong with my method ard man?

[Luke]

thanks everyone ;)

+rep to everyone that posted code if i can.

Just so you know, all the code etc and stuff i've been posting latley i'm making a mini private CMS for my local ATC.

So far, i've managed, a mini profile system (need to set some kind of layout lol), mini news cms, download cms, rank system (shows what rank the person is on their profile), and alert system.

I'm getting hang of it, i see realy how easy php is if you study it ;P
Thanks ;)
Luke

[Protege]

No problem sxi thanks for rep.

Found a picture of Charlie the CEO of WeBeg PLC, i'd be careful: http://img.4chan.org/b/src/1219173417951.jpg

[Hypertext]

if( mysql_num_rows( $mysqlQuery ) > 0 )

What was wrong with my method ard man?
Two comparisons are slower than one.

[Protege]

So your saying if its slower, theres no point using it?

Its like you still trying to beat people @ the 100m sprint, it aint gunna happen.