On Sat, 10 May 2008 01:33:19 +0200, JRough <jlrough@yahoo.com> wrote:
> I am trying to organize a long page of php code I inherited.

comp.lang.php was to hard to find?

> It
> mostly has includes and if statements and it is hard to read. I want
> to take this function off the page. It gets called only twice:
> getsubtree($catsubtree,$i +1);

$catsubtree & $i being what?

> getsubtree($catsubtree($catsubtree,0);

This is a syntax error, this code would never work. For sake of argument,
I'll assume this is just a typo duplicating '$catsubtree(', and this
actial call is 'getsubtree($catsubtree,0)'. For my benefit, it would also
help if the only time 'getsubtree($catsubtree,$i +1)' is called is the
recursion in the getsubtree function itself.


> My question is how do I do that? I put the function on a separate
> page and name it for example getsubtree.php. Then I put the
> following at the top with the other includes:
> include $include_path.'getsubtree.php'; Is that right?

Euhm, I usually put all usefull functions in one file, classes have their
own file though, as they tend to be more elaborate and needed less often,
and it helps transparancy. Wether your include statement is correct
depends on what $include_path holds.

> //
> function getsubtree($catsubtree,$i) {
> global $catlist;

Function relying on existing globals is something you want to avoid. If
$catlist is of importance, try to get it to be passed as an argument or
reference to the function.

> Do you have an idea of what the function is doing?

Let's try it stament by statement, all of these or obviously (educated)
guesses, as we don't see either you database scheme, current output, or
relating code:

> $res=mysql_query("SELECT * FROM PHPAUCTIONXL_categories WHERE
> parent_id=".$catsubtree[$i]);

= Get all categories which are a child from the category which is in
$catsubtree[$i].
Remarks:
- Probably, the function has no business trying to match $catsubtree &
$i to $catsubtree[$i]. If the relation is that obvious, it should either
be maintained elsewhere or passed directly as $catsubtree[$i].
- SELECT * should never be used in production, name the columns you need

> while($row=mysql_fetch_assoc($res)) {

Loop through all the categorie properties as defined in
PHPAUCTIONXL_categories.
Remarks:
1) Before a loop, one should check wether $res is even a valid resource.
2) Probably the recursion mentioned later is stopped here by just having
$res not being a valid result as $catsubtree[$i] is no longer set.

> if(mysql_num_rows(mysql_query("SELECT cat_id from
> PHPAUCTIONXL_varlabels WHERE cat_id=".$row['cat_id']))==0) {

Get the number (mysql_num_rows) or entries in PHPAUCTIONXL_varlabels where
the cat(egory)_id is equal to the current child of $catsubtree[$i], and
perform the following actions if the number of entries is 0:
Remarks:
- Feeding a mysql_query result directly to mysql_num_rows is just silly,
one could allready COUNT() in MySQL, and fetch the resulting column if
needed.

> // get info about this parent
> $catlist[]=$row['cat_id'];

The current cat_id from the child, which has NO entries in
PHPAUCTIONXL_varlabels, will be added to the global $catlist variable.

> $catsubtree[$i+1]=$row['cat_id'];

Either insert or replace (if it allready exists) an entry in $catlist with
the key $i+1 with the current childs cat_id.

> getsubtree($catsubtree,$i+1);

For every entry in PHPAUCTIONXL_categories which is a child of
$catsubtree[$id] and had no entries in PHPAUCTIONXL_varlabels, repeat for
the entry in $catsubtree with an index 1 higher than the current index
(which BTW is the index you just added or replaced). Is this the second
call you mentioned BTW?

> }
> }
>


It really is messy code, no wonder you ask what it does.... comp.lang.php
would be the far better option. However, not being sober I'll answer you
here:

As far as I can tell:
- The $catsubtree argument is an array (I hope so, as otherwise [] would
produce quite different results then expected..) with category id's. How
this is created is a mystery to us.
- $i is the index in $catsubtree we want information off
- we put children categories of the

What I suspect:
- PHPAUCTIONXL_categories is an hierarchical list of categories.
- cat_id is an auto_increment primary key in PHPAUCTIONXL_categories
- parent_id is the id of the parent category, and it is 0 instead of a
more obvious NULL if the current category has no parent (i.e. is a root
category).
- the code tries to build an ordered list of root categories and it's
children which have no entries in PHPAUCTIONXL_varlabels (the reason for
that last part is not totally clear to me) in a global variable $catlist
similar to this (cat_id's replaced with somewhat illustrative names):
$catlist = array(
'root1',
'sub1',
'subsub1',
'subsub2',
'sub2',
'subsub3',
'root2',
'sub3',
'subsub4');
etc...


I suggest using this more transparant code (be aware you newsreader
_might_ insert linebreaks which break the mysql query in there, the
comment should all be after the # on the same line):


//function definition
function getsubtree($i = 0,$list = array()){
//$list should always be an array:
if(!is_array($list)) trigger_error("getsubtree(): second parameter
should be an array or omitted",E_USER_FATAL);
//get all categories from PHPAUCTIONXL_categories
//where the parent is $i
//and which has no entries in PHPAUCTIONXL_varlabels
//some PHP 'magic' transferred to MySQL 'magic'

//I suspect $i is an integer, but just be sure,
//I'll assume a possibly 'evil' string:
$i = mysql_real_escape_string($i);
$result = mysql_query("
SELECT cat_id #select the id
FROM PHPAUCTIONXL_categories c #from categories
LEFT JOIN PHPAUCTIONXL_varlabels vl #but check the varlables
ON vl.cat_id = c.cat_id #for the selected category
WHERE c.parent_id = '$i' #which should be a child of
category #$i
vl.cat_id IS NULL #and only select categories
without varlables");

//if the query fails, return
if(!$result) return $list;
//fetch existing children if needed:
while($row = mysql_fetch_assoc($result){
//add the category to the list:
$list[] = $row['cat_id'];
//check this category for children, and possibly add them to the list
//also known as recursion of course:
$list = getsubtree($row['cat_id'],$list);
}
//return the list as build now
return $list;
}
//don't use a global if not needed, so replace the call
//'getsubtree($catsubtree,0);' with:
$catlist = getsubtree(0);

I have checked abolutely 0 workability of this code, but I think it quite
clearly illustrates a better way to do the thing that the given code does,
and providing the added bonus of the possibility of retrieving ordered
subtrees instead of the whole ordered tree if needed. Even giving the
returning $list entries information about the depth of the current item is
possible if you add '$depth=0' as a last argument to the function
definition, alter the adding to '$list[] = array('id' =>
$row['cat_id'],'depth' = $depth), and the recursion to '$list =
getsubtree( $row['cat_id'] , $list,$depth+1);'. Any using code of the
resulting list would obviously have to be altered to deal with arrays
rather then scalars in that list.

For a better understanding of some hierarchical lists in MySQL, reading
<http://dev.mysql.com/tech-resources/articles/hierarchical-data.html> is
not a bad thing, and possibly even a a better solution ig the possible
depth of the tree is limited. If you want to know more, Joe Celko's Trees
and Hierarchies in SQL for Smarties is _the_ book to read IMHO.

Follow up set to comp.lang.php, as most of this really isn't a MySQL issue.
--
Rik Wasmus
[SPAM] Now temporarily looking for some smaller PHP/MySQL projects/work to
fund a self developed bigger project, mail me at rik at rwasmus.nl. [/SPAM]