Is my code "clean" enough?

[schwim]

Hi there guys,

I'm sorry to bother everyone, but I've been teaching myself php by disassembling other scripts, and taking the parts I need to make the script I want to write. I search Google for what I don't understand, and then view php.net for help with arguments on the functions, but there's still a lot that I don't understand, and I KNOW that there's a lot I'm doing wrong.

I wanted to get the count of a DB table, and print the outcome for a "statistics" page on the testimonials admin section, and I found a section of a forum script that showed the count of posts in the forum index, and I used that to come up with the following. It seemed like a lot of script for 3 simple numbers, so I thought that maybe I'm doing this in a very inefficient way. I thought that maybe I can get all three numbers by querying the DB only one time.

Can someone tell me if this an acceptible way to show: total, approved & waiting in a table? Also, although the forum kept using the same var, and filling it with each new category count, I changed each one. I shouldn't keep the variable name the same in each function, should I($num for total, $num for approved and $num for waiting)?

		        echo ("
		        <p><b>Current statistics:</b></p>
		        <ul>
		        <li>Total Testimonials: ");

	        $query = mysql_query("SELECT * FROM testimonials");
			$total = mysql_num_rows($query);
			if($total==0)
			{
				echo ("0");
			}else{
				echo $total;
			}

	        echo ("</li>
	        <li>Approved: ");

	        $query = mysql_query("SELECT * FROM testimonials WHERE active=1");
			$total_app = mysql_num_rows($query);
			if($total_app==0)
			{
				echo ("0");
			}else{
				echo $total_app;
				echo (" (<a href=index.php?action=approved><i>view</i></a>)");
			}

	        echo ("</li>
	        <li>Waiting: ");

	        $query = mysql_query("SELECT * FROM testimonials WHERE active=0");
			$total_wait = mysql_num_rows($query);
			if($total_wait==0)
			{
				echo ("0");
			}else{
				echo $total_wait;
				echo (" (<a href=index.php?action=waiting><i>view</i></a>)");
			}

	        echo ("</li>
	        </ul>
	        ");

Thank you all very much for your time,
json

[halojoy]

I shouldn't keep the variable name the same in each function,
should I ($num for total, $num for approved and $num for waiting)?

It is possible to use same $num variable name, in different functions
as long as this variable name is local

not when it is global like this

function counter(){
global $num;

//
//
}

[Weedpacket]

Although in this case it would be better to use the database to do what the database is good at, and PHP for what it is good at...

<?php
	// I'm assuming that "active" might take values other than 0 and 1...
	// Much simplification possible if it can't.
	$query = "
	Select count(*) from testimonials
	union
	Select count(*) from testimonials where active=1
	union
	Select count(*) from testimonials where active=0";

$result = mysql_query($query);
$total = mysql_result($result, 0);
$approved = mysql_result($result, 1);
$waiting = mysql_result($result, 2);
?>
                <p><b>Current statistics:</b></p>
                <ul>
                <li>Total Testimonials: <?php echo $total?></li>
                <li>Approved: <?php echo $approved?><?php
	if($approved>0)
		echo " (<a href="index.php?action=approved"><i>view</i></a>)"; ?></li>
                <li>Waiting: <?php echo $waiting?><?php
	if($waiting>0)
		echo " (<a href="index.php?action=waiting"><i>view</i></a>)"; ?></li>
	</ul>

[Shrike]

Hmm, should be even more simple than that...

SELECT active, COUNT( active )
FROM table
GROUP by active

...No? 🙂

[schwim]

Hi there Weedpacket,

I'm writing for MySQL 3, so I can't use your example, correct?

Hi there Shrike,

How would I actually implement your suggestion? That's the statement to pull the data, but how do I print the three variables(total, waiting & approved)?

thanks to both for your time,
json

[cahva]

Mysql 3 is STILL in use somewhere? 🙂

[schwim]

If not anywhere else, at least on 4 of the seven servers I work with.

Since they're the ones I work with, they're really the only ones that concern me in regards to my script working 😃

thanks,
json

[Weedpacket]

Don't look at me. I don't use MySQL at all: I'm just guessing on the basis of what the SQL standard says. (That said, I'd go along with Shrike's version: I've also been mucking around with SQL Server's recursive CTE's too much lately and had union on the brain.)

As for getting the data out of the result set; it's just a result set. PHP offers several functions (the most convenient in this case would be [man]mysql_fetch_row[/man]. The first element of each row fetched would be a possible value of active, the second element would be a count of the number of records where active has that value. Obviously, you'll need to add together the counts to get the total, but that's just adding numbers together.