Topic awaiting preservation: PHP, operators, and some really ugly code

Hello again,

You guys saved me last time with something that was obvious, so I hope this isn't quite so stupidly simple.

I am trying to create code in PHP to record a visitor's IP as they visit a page. I would like to simply use the same code for each page and have the info dump into a MySQL table and database. Now I managed to stitch together a simple script that captured every hit to a page and add a new line for each page, however, I'm having trouble getting the unique IP one working.

<?php



include( 'includes/header.php');



$vari = "SELECT id, page, date, ip, count, browser, referal from ip_hits";



$page = $vari['page'];

$pageip = $vari['ip'];

$count = $vari['count'];



$ip = getenv ("REMOTE_ADDR");

$httpref = getenv ("HTTP_REFERER");

$httpagent = getenv ("HTTP_USER_AGENT");



$update = "UPDATE ip_hits SET count=count + 1, date=now() WHERE page = '$PHP_SELF' && ip = '$ip'";

mysql_query($update);

if (($page <> $PHP_SELF) && ($pageip <> $ip)) { 

	$result = "INSERT INTO ip_hits VALUES ('', '$PHP_SELF', now(), '$ip', 1, '$httpagent', '$httpref')"; 

	mysql_query($result);

}

?>

Now, the problem is probably very obvious to you all, however, when I use this code, it updates any table row that has both the page and the IP, which is exactly how I intended the code to work. However, it still adds a new line of code on each reload of the page. So my question is: What is wrong with my operators after the 'IF' statement that causes the code to add another row to the table? From my interpretation of the code, it should only insert a new row IF both the page AND ip are NOT in the same row.

Help! ... Please! ::grin::

Thank you all,

Dustin

P.S. - Again, my apologies about the ugly code, I do hope to eventually get better. ::grin::

Hello,

You forgot to execture the SQL query stored in $vari, and this SQL query has no WHERE clause. Therefore the variables $page, $pageip and $count are not initialized, and f**k up the condition in your if( ... ) statement


Actually I think you could remove the code before the initialization of $ip, and replace the condition in you if statement by mysql_affected_rows()==0. That way you would insert a record only if the update failed.


Btw, that sort of hit logs is really space consuming. To the minimum you should consider storing the userAgent strings and page URL in two seperate tables and only use foregin keys in the ip_hits table.

Hope that helps,

poi Thank you!

That whole " myql_affected_rows" statement worked like a charm. I removed the un-needed sql query from the top as well.

I warned ya that it would be ugly code. I'm trying to figure all this stuff out as I discover an immediate need for it, so sometimes I'm adding stuff in that later I discover is not needed.

I do have another question, and it is why this table setup is more space consuming that two tables? I'm not a database designer by any stretch of the imagination, so this part of your post eludes me. I would think that having all information in one table would be more efficient to call from, than sending requests to several different tables simultaniously (sp?). Is there more load on the database by this one request rather than multiple requests? Am I anywhere near the ballpark in this guess? ::grin::

Anyway, you helped big time

Dustin

[quick_reply]

With a single table it's more space consuming because you'll store many redundant datas.
For instance, the values of the page field will be repeated an huuuuge number of times, so will be the ip and user_agent. Therefore, if you have a separate table for the page, you'll only store their name once, and will replace it by a foregin key in the ip_hits table.

Indeed it's faster to store all these infos in one table, but since you access them only once per page and that the size of the table will grow really fast, you should consider, before it becomes painfull to do, to save some space.

Actually if you can do all you stats things in a separeted DB it'd be better and wouldn't slow down your main DB.

[/quick_reply]