request for help cleaning up very small php code

i have a mysql db table on godaddy with 5 fields: id, linkURL, graphic, impressions, clicks

using a php script downloaded from http://www.phpjabbers.com/free-banner-and-link-click-counter-script, i am able to record the clicks perfectly but i had to adapt things a bit in order to grab the graphic addresses and record the graphic impressions. (my banner graphics, with different links, are side-by-side.)

what i have works fine, but is a mess, i am certain.

could someone please tell me how to clean up the code:

<?php error_reporting(0); include("config.php");

$sql = “SELECT * FROM “.$SETTINGS[“data_table”].” WHERE id=19”;
$sql_result = mysql_query ($sql, $connection ) or die ('request “Could not execute SQL query” '.$sql);
if (mysql_num_rows($sql_result)>0) {
while ($row = mysql_fetch_assoc($sql_result)) {
$sql = “UPDATE “.$SETTINGS[“data_table”].” SET impressions=impressions+1 WHERE id=19”;
$sql_result = mysql_query ($sql, $connection ) or die ('request “Could not execute SQL query” '.$sql);
echo “<img src=”.$row[“graphic”]." />";
}
}
?>

<?php
$sql = “SELECT * FROM “.$SETTINGS[“data_table”].” WHERE id=20”;
$sql_result = mysql_query ($sql, $connection ) or die ('request “Could not execute SQL query” '.$sql);
if (mysql_num_rows($sql_result)>0) {
while ($row = mysql_fetch_assoc($sql_result)) {
$sql = “UPDATE “.$SETTINGS[“data_table”].” SET impressions=impressions+1 WHERE id=20”;
$sql_result = mysql_query ($sql, $connection ) or die ('request “Could not execute SQL query” '.$sql);
echo “<img src=”.$row[“graphic”]." />";
}
}
?>

thank you very much!

A lot of that could be put on 1 line, such as
[php]$sql_result = mysql_query(“SELECT * FROM “.$SETTINGS[“data_table”].” WHERE id=19”) or die(mysql_error());
if (mysql_num_rows($sql_result)>0) {
while ($row = mysql_fetch_assoc($sql_result)) {
$sql_result = mysql_query(“UPDATE “.$SETTINGS[“data_table”].” SET impressions=impressions+1 WHERE id=19”);
echo “<img src=”.$row[“graphic”]." />";
}
}// not sure what goes to. You have it in the second example too.[/php]

thanks for your quick reply.

since i am calling the left banner and right banner addresses from their row’s respective “graphic” fields in the database (and tracking the number impressions for each), is there a way to consolidate the code for the two calls, rather than repeating the whole thing twice, as i am doing?

thanks again!

I could be quite off the mark here as i haven’t read the code below but going off this answer, my response was put the code in a function.

Having paused, and now read the code i see you would need to pass in the id and return the img.
(and also rejig the code a little)
Hope that makes sense.

thank you again.
i sure appreciate your patience.

unfortunately, i am in the weeds about how to implement your suggestion.
if we forget about the link and think of my predicament as:

  1. i have a table with only 3 fields, called: id, graphics (that holds the url to an image) and impressions.
  2. i call two static images per page (in my sample bit of code, this would be rows 19 and 20 from the graphics field) and these two images display side by side on the web page.
  3. i want to track the number of impressions of each graphic.

what would be the most efficient way to pull this off?

thank you again.
i really appreciate your help!

Ok, this code is written from the top of my head so may have a bug or two but on the whole you should get the idea.

[php]function select_image($id, $connection, $SETTINGS) {
$sql = “SELECT * FROM “.$SETTINGS[“data_table”].” WHERE id=(UPDATE “.$SETTINGS[“data_table”].” SET impressions=impressions+1 WHERE id=” . $id . “)”;
$sql_result = mysql_query($sql, $connection ) or die ('request “Could not execute SQL query” '.$sql);
return “<img src=”.$row[“graphic”]." />";
}

$img1 = select_image(19, $connection, $SETTINGS);
$img2 = select_image(20, $connection, $SETTINGS);
[/php]

The best advice i can give you today is stop using mysql !! :’( :’(
Use mysqli or PDO instead.

Red :wink:

Note: I edited my post so as to pass in the $connection, and $SETTINGS arguments also.
(you could make them global, but i hate globals so try to avoid them, i only mention it here to give you another option to passing in the last two args.)

drats…

here is the error message i am getting now:

request “Could not execute SQL query” SELECT * FROM banner_clicks WHERE id=(UPDATE banner_clicks SET impressions=impressions+1 WHERE id=19)

here is how i entered your chunk of code:

<?php error_reporting(0); include("config.php"); function select_image($id, $connection, $SETTINGS) { $sql = "SELECT * FROM ".$SETTINGS["data_table"]." WHERE id=(UPDATE ".$SETTINGS["data_table"]." SET impressions=impressions+1 WHERE id=" . $id . ")"; $sql_result = mysql_query($sql, $connection ) or die ('request "Could not execute SQL query" '.$sql); return ""; } $img1 = select_image(19, $connection, $SETTINGS); $img2 = select_image(20, $connection, $SETTINGS); ?>

change this line:
[php]die ('request “Could not execute SQL query” '.$sql);[/php]
to this:
[php]die ('request “Could not execute SQL query” '.mysql_error($connection));[/php]

for a more detailed error output and post the results back…

ps: also: error_reporting(1);

here is the new error:

request “Could not execute SQL query” You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ‘UPDATE banner_clicks SET impressions=impressions+1 WHERE id=19)’ at line 1

try this, change:
[php]SELECT * FROM[/php]
to this:
[php]SELECT graphics FROM[/php]

Looking at the function again i see i missed out a line - a crucial one too although not connected to the errors you’re seeing.

replace:
[php]return “<img src=”.$row[“graphic”]." />";[/php]
with:
[php]$row = mysql_fetch_row($sql_result, MYSQL_ASSOC);
return “<img src=”.$row[“graphic”]." />";[/php]

PS: It’s 02:15am here so i’m gonna be hitting the hay soon so if I don’t reply I will take another look when i get up.

dang…
same error message.
where you mentioned to change * to graphics, i assume you meant to say graphic (singular) since that is the field name.
here is my code:

<?php error_reporting(1); include("config.php"); function select_image($id, $connection, $SETTINGS) { $sql = "SELECT graphic FROM ".$SETTINGS["data_table"]." WHERE id=(UPDATE ".$SETTINGS["data_table"]." SET impressions=impressions+1 WHERE id=" . $id . ")"; $sql_result = mysql_query($sql, $connection ) or die ('request "Could not execute SQL query" '.mysql_error($connection)); $row = mysql_fetch_row($sql_result, MYSQL_ASSOC); return ""; } $img1 = select_image(19, $connection, $SETTINGS); $img2 = select_image(20, $connection, $SETTINGS); ?>

sorry for delay had to build a database to test the code…

This works:
[php]function select_image($id, $connection, $SETTINGS) {
//
$sql = “SELECT id, graphic FROM " . $SETTINGS[‘data_table’] . " WHERE id=” . $id . “”;
$sql_result = mysql_query($sql, $connection) or die ('request “Could not execute SQL query” '.mysql_error($connection));
$row = mysql_fetch_row($sql_result, MYSQL_ASSOC);
$img = ‘<img src="’ . $row[“graphic”] . ‘" alt="" />’;
$sql = “UPDATE " . $SETTINGS[‘data_table’] . " SET impressions=impressions+1 WHERE id=” . $id . “”;
$sql_result = mysql_query($sql, $connection) or die ('request “Could not execute SQL query” '.mysql_error($connection));
$row = mysql_fetch_row($sql_result, MYSQL_ASSOC);
return $img;
}

echo $img1 = select_image(19, $connection, $SETTINGS);
echo $img2 = select_image(20, $connection, $SETTINGS);
[/php]

I’m off to bed, i’ll check in tomorrow see how you got on.
Red :wink:

no luck.

this time, i get no error messages, no graphics, nothing.

it is as if the code isnt even there.

<?php error_reporting(1); include("config.php"); function select_image($id, $connection, $SETTINGS) { $sql = "SELECT id, graphic FROM " . $SETTINGS['data_table'] . " WHERE id=" . $id . ""; $sql_result = mysql_query($sql, $connection) or die ('request "Could not execute SQL query" '.mysql_error($connection)); $row = mysql_fetch_row($sql_result, MYSQL_ASSOC); $img = ''; $sql = "UPDATE " . $SETTINGS['data_table'] . " SET impressions=impressions+1 WHERE id=" . $id . ""; $sql_result = mysql_query($sql, $connection) or die ('request "Could not execute SQL query" '.mysql_error($connection)); $row = mysql_fetch_row($sql_result, MYSQL_ASSOC); return $img; } echo $img1 = select_image(19, $connection, $SETTINGS); echo $img2 = select_image(20, $connection, $SETTINGS); ?>

ok, i know that function works as i built and tested it on my system (including a database with images.) - although the second mysql_fetch_row is not needed.

That means something else must be causing problems.
Post up the code from config.php let me take a nosey.

Red :wink:

thank you for sticking with me on this.
here is the config file i am calling:

<?php /* Define MySQL connection details and database table name */ $SETTINGS["hostname"]='rinasheville.db.9276399.hostedresource.com'; $SETTINGS["mysql_user"]='rinasheville'; $SETTINGS["mysql_pass"]='hidden'; $SETTINGS["mysql_database"]='rinasheville'; $SETTINGS["data_table"]='banner_clicks'; /* Connect to MySQL */ if (!isset($install) or $install != '1') { $connection = mysql_connect($SETTINGS["hostname"], $SETTINGS["mysql_user"], $SETTINGS["mysql_pass"]) or die ('Unable to connect to MySQL server.

Please make sure your MySQL login details are correct.'); $db = mysql_select_db($SETTINGS["mysql_database"], $connection) or die ('request "Unable to select database."'); }; ?>

What is $install and where is it set?

change the following:[php]
if (!isset($install) or $install != ‘1’) {
$connection = mysql_connect($SETTINGS[“hostname”], $SETTINGS[“mysql_user”], $SETTINGS[“mysql_pass”]) or die (‘Unable to connect to MySQL server.

Please make sure your MySQL login details are correct.’);
$db = mysql_select_db($SETTINGS[“mysql_database”], $connection) or die (‘request “Unable to select database.”’);
};
[/php]

to this:
[php]
$connection = mysql_connect($SETTINGS[“hostname”], $SETTINGS[“mysql_user”], $SETTINGS[“mysql_pass”])
or die (‘Unable to connect to MySQL server.

Please make sure your MySQL login details are correct.’);
$db = mysql_select_db($SETTINGS[“mysql_database”], $connection)
or die (‘request “Unable to select database.”’);
[/php]

let me know what happens.

… and here is the structure of the table:

Field Type Collation Attributes Null
id int(11) No auto_increment
url varchar(250) utf8_unicode_ci No
clicks int(11) No
sourcepage varchar(255) utf8_unicode_ci No
impressions varchar(5) utf8_unicode_ci No
graphic varchar(255) utf8_unicode_ci No

thanks!

isset() is part of the initial code (for counting graphic clicks) that i downloaded from http://www.phpjabbers.com/free-banner-and-link-click-counter-script

unfortunately, those downloaded files didnt have a way to track impressions, so that is why i started adding on to what they already had.

to count clicks you would need to use javascript.

Php and javascript are two completely different beasts altogether and any variables you set using PHP will not be picked up by javascript - and vice versa.

Let me see what i can come up with…

thank you.
once again, that hacked-up code that i posted in the first message of this thread works fine (it displayed the graphics and counted impressions and clicks) but i am just certain that the coding is awfully sloppy.

Sponsor our Newsletter | Privacy Policy | Terms of Service