recursive cat/subcategory function

NJphpKid

I wrote a section of code to print out categories and subcategories by using a combination of mysql queries, recursive function and arrays to store both all values and a list of cat_id's to keep track of the depth.

Please look this over and let me know what you think. Especially if you think I missed any major points. I will start to work the admin side (add/delete cats, subcats safely) of this and possibly make it into a class.

The DB structure used is like so:
cat_id (primary key)
subcat_id
cat

I used a database abstraction layer ez_sql (link is in code). If the subcat_id==0, then it is a first layer cat and is put in bold. If one record's subcat_id == to another cat_id then it is a subcat of that cat.

The code can be found here (in a textarea box)
http://aarond.com/am/view_cats.php

axo

first small thing:

#---- poor code
   // indent maker loop
   $temp_subcounter=$subcounter;  

   while($temp_subcounter!=0){   // to preserve global value
	  print "  ";        // print indent (can change to image if necessary)
    $temp_subcounter--;          // decrease tempvar each time
	 } // end while
#---- poor code end

#---- better and faster code
echo str_repeat(" ",$subcounter);
#---- better and faster code end

second:

a function MAY NOT print or echo. a function RETURNS values.

third:

recursion and mysql (or any other relational database system) don't fit very well.
just think of what will happen if you have a depth of more than 10 in your 'tree'... the number of database queries will explode.

try using 'nested sets' -> search google for those.

fourth:
it's very nice that you commented your code - there are many people who don't do it and that's no good. but, in my opinion, you don't need to comment all those closing brackets // end foreach , // end function , // end while - i feel it makes code quite harder to read than if you would have left the brackets uncommented. just comment tricky lines, lines which you would later improve or just lines which would be hard to read if you didn't comment them...

happy coding!

Weedpacket

Originally posted by axo
a function MAY NOT print or echo. a function RETURNS values.

Why not? Certainly it's nice to separate the two tasks if you're planning on using the result for anything other than display but what's with the capital letters?

recursion and mysql (or any other relational database system) don't fit very well.

Very true; consider selecting first (with one query - which in fact you've already done!) and then recursing on the returned rows.

NJphpKid

Thanks for the feedback!!

I wasn't aware that functions shouldn't print anything. Does it slow down the script?

I'm reading up on 'nested sets' (thank you!) It might take me a little while to figure out what's going on.

Here is my second round of code now with:
-only 1 query to the database
-using str_repeat() function to indent cats
-a for loop in the recursive function to search for subcats

 
// page to display cats/subcats in tree formation
// Uses the ez_sql Database abstraction layer found here:
// jvmultimedia.com/home/articles.php?articleId=2
// ---------------------------------------------------------
include("ez_sql.php"); // call database connect layer

// sample array name/value
/* print $all[0][0]; // cat_id
    print $all[0][1]; // subcat_id
    print $all[0][2]; // cat name       */

// vars
$all=$db->get_results("SELECT * FROM gal_cat", ARRAY_N);  // put all cats into array
$cat_id_array=array(); // push/pop cat id's 
$subcounter=0;         // to count what subcat level function is on, used for indenting

// recursive function to print second level cats and subcats
// ........................................................
function SubCat($id, $cat_id_array, $subcounter){
 global $subcounter;
 global $db; 
 global $all; 

 // find all cats w/given subcat_id
 for($a=0; $a<count($all); $a++){
  if($id == $all[$a][1]){
   $indent=str_repeat("&nbsp;&nbsp;", $subcounter); // indent
   print $indent.$all[$a][2]."<BR>\n";              // print subcat
   array_push($cat_id_array, $all[$a][0]);          // push cat_id to array 
	 																		              //  (now have string of cats to go back through)

 // recursively call SubCat function for subs of this id & increment subcounter
 SubCat($all[$a][0], $cat_id_array, $subcounter++);	 
   } // end if
 } // end for

  array_pop($cat_id_array); // pop cat_id off array when done with it
	$subcounter--;	          // decrement subcounter
} // end subcat function
// ........................................................


// STARTS HERE: Loop & print through all top level cats
for($i=0; $i<count($all); $i++)  

{
 if($all[$i][1]==0){                      // if subcat_id=0
  print "<b>".$all[$i][2]."</b><BR>\n";   // print topcat 
	array_push($cat_id_array, $all[$i][0]);	// push cat_id to array

  // subs : go through remaining records for subcats of last cat_id_array value
  $last=count($cat_id_array)-1; // because of zero key
  SubCat( $cat_id_array[$last], $cat_id_array, $subcounter=0 ); // print subcats if any	 	
  print "<BR>";	
 } // end if  

} // end for

Weedpacket

Originally posted by NJphpKid
I wasn't aware that functions shouldn't print anything. Does it slow down the script?

No, it just means you can't use the function without it printing (if, e.g., you wanted to write its output to a file instead) without either rewriting the function or doing more work (output buffering) that would slow down performance.

Beyond that, there's no reason why you shouldn't or mustn't or can't or oughtn't or mayn't.

axo

me personally thinks it better to disallow some things like the "breaking up of the functionality" to beginners. i think it is essential to get to know functions in their original, mathematical sense: input -> output; function domain -> codomain. with the learning of this just a bit more abstract thinking, it's much easier to understand OOP later on, and i think beginners will make less faults when they get used to the language and are to solve more complex problems or have to learn some more complex language like c++ or java.

the breaking up of rules should, in my opinion, be reserved for those who know what they are doing.

Weedpacket

Originally posted by axo
the breaking up of rules should, in my opinion, be reserved for those who know what they are doing.

You're contradicting yourself, then, insisting that side effects like output should be broken out of functions.

Besides, mathematical functions and the functions of a programming language like PHP are not equivalent: the latter aren't necessarily "well-behaved". And mathematical functions don't have side effects.