Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

General question about variable $current_category_id


lagodilecco

Recommended Posts

I'm sure I'm going to end up looking stupid, but I can't help myself. I just HAVE to know!:blink:

 

I was looking through the code for one of the contributions, and it has code which goes something like this (I'll put the actual code at the end later):

 

select c.categories_id from TABLE_CATEGORIES c ... where c.categories_id = (int)$current_category_id ...etc...

 

In other words, the value returned MUST be the same as $current_category_id, even with all the other stuff which is in the full SQL statement.

 

So, here's where I risk looking stupid. Can anybody tell me WHY they bother doing this as a database lookup at all? If all they want is a category id, don't they already have the category id? Please put me out of my misery. I will forgive you for making me look stupid. Promise.

 

Here's the actual code (it's from the subcategory description contribution):

 

<?php $text_query = tep_db_query("select c.categories_id from " . TABLE_CATEGORIES . " c, " . TABLE_CATEGORIES_DESCRIPTION . " cd where c.categories_id = '" . (int)$current_category_id . "' and cd.categories_id = '" . (int)$current_category_id . "' and cd.language_id = '" . (int)$languages_id . "'");

$text = tep_db_fetch_array($text_query);

?>

 

The SQL does join two tables, but you can't escape that the c.categories_id returned MUST equal the $current_category_id variable. No matter what the languages_id...?

 

O.K, do your worst - tell me that I'm mad!

 

P.S Yes I do know SQL (though Oracle...) and yes I have broken up this code and tested it. Everything is as I expected...

Link to comment
Share on other sites

Looks like a query that is not needed. Contributions are made by all levels of coders, from day 1 coders to experienced coders, so not all contributions are coded equally.

Link to comment
Share on other sites

Thanks for that. Maybe I'm not mad, then.

 

My testing does seem to confirm that the same purpose is achieved by simply referring directly to $current_category_id, no SQL involved. It does vindicate a cautious approach to contributions.

Link to comment
Share on other sites

Also, note that experienced coders were once day 1 coders. I cringe at some of my old stuff lol

 

I don't know which contribution you are referring to, but it does seem strange to do this:

 

$text_query = tep_db_query("select c.categories_id from " . TABLE_CATEGORIES . " c, " . TABLE_CATEGORIES_DESCRIPTION . " cd where c.categories_id = '" . (int)$current_category_id . "' and cd.categories_id = '" . (int)$current_category_id . "' and cd.language_id = '" . (int)$languages_id . "'");

$text = tep_db_fetch_array($text_query);

 

Basically that are saying that the variable $text = current_category_id as you pointed out - it is possible that what they want to do is say $text = the description of the current category. Mistake in the SQL maybe?

 

select c.categories_description ??

Link to comment
Share on other sites

No, they really are trying to test the category id (it's the "Sub Category Description on Selected Categories" 4608 contribution - I have posted a query/suggestion on the support thread).

 

The next lines are actually:

<?php if ($text['categories_id'] == '28') { ?>

<tr><td><?php echo SUB_CAT_DESC_28; ?> <br><br></td></tr>

<?php }

?>

 

In my testing, I've replaced the whole lot (including database looking) with:

<?php if ((int) $current_category_id == 28) { echo "<tr><td>Description for category 28...<br><br></td></tr>"; } ?>

 

Or even:

<?php if ($current_category_id == '28') { echo "<tr><td>Description for category 28...<br><br></td></tr>"; } ?>

 

Both work (though I think it's probably safer with the cast to int).

 

I've scratched my head and can't see why the database lookup is necessary, given that it MUST return the same value as $current_category_id.

 

Thanks for your help and interest (and love your site, I should probably buy you a beer!).

Diana

 

 

 

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...