affiliate counter

[Manat]

Hello all,

We will soon be having a banner on another site linking to ours.
I gave them an AFID=3243 query string.

I have a little counter that counts the number of users coming to our site with that affiliate ID.

Please critique my code and let me know if you would do anything differently.

Thanks.

<?php
$result = mysql_query ("SELECT * FROM counter");
//$rows = mysql_numrows($result);

for ($count = 1; $row = mysql_fetch_row ($result); ++$count)
{}
$now = getdate();
$month = $now[mon];
$mday = $now[mday];
$year = $now[year];
$hours = $now[hours];
$min = $now[minutes];
$sec = $now[seconds];
$id = $count++;
$browser = "$HTTP_USER_AGENT";
$referer = "$HTTP_REFERER";
$time = "$year-$month-$mday $hours:$min:$sec";
for ($i=0; $i < strlen($id); $i++)
{
$sign = substr($id, $i, 1);

}

if ($referer != "$PHP_SELF")
{
if ($afid == 3243)
{
$counter = "INSERT INTO counter (ID, browser, referer, time, afid) VALUES ('$id','$browser','$referer','$time','$afid')";
$result = mysql_query($counter);
}

}
?>

I put the if referer not equal to self so that refreshes don't count as an extra visit.

Let me know if this a good solution to affiliate tracking/counting.

Thanks
MK

[Weedpacket]

First of all, I'd post the code in [php] tags and indent it so that it's easier to read. (See, I told ya!)

Second, I'd quote the associative array keys.

Third, I'd use SELECT COUNT(*) and return only the information I want from the table (the number of rows it contains), instead of the entire table.

Fourth, personally, I'd use date() instead of getdate()

Fifth, I'd be more likely to use $_SERVER['HTTP_USER_AGENT'] etc., instead of $HTTP_USER_AGENT.

Sixth, wrapping a single string variable in double quotes (e.g., "$HTTP_USER_AGENT") just wastes time: it creates a new string, searches it for variables, interpolates those variables with their string values, and returns the resulting string .... which in this case happens to be exactly the same as what the variable's string value was in the first place.

Seventh, since $sign is never used, the loop around it is a do-nothing. Maybe it's used elsewhere, but it's going to be the last character in the $id anyway, so the loop could be replaced with $sign=substr($id,-1);.

Eighth, if the id is an incrementing counter for the purposes of uniquely identifying rows, then MySQL (and most other DBMSs) have mechanisms for specifying such fields automatically (MySQL has AUTOINCREMENT, PostgreSQL has SERIAL, MS SQL Server has IDENTITY...)

Ninth, if the fields are numeric, then they oughtn't be quoted; if they're all strings then that will cause problems when ordering (because '10'<'2').

Tenth; what's $afid? Where does that come from? Oh, the querystring. So therefore it ought to be $_GET['afid'].