Jump to content



Latest News: (loading..)

- - - - -

Little bit of code cleanup help

code

This topic has been archived. This means that you cannot reply to this topic.
8 replies to this topic

#1   douglaswalker

douglaswalker
  • Members
  • 31 posts

Posted 27 April 2012 - 02:28 PM

Hi there
the piece of code below is from product_info.php. It is to show an instock out of stock sign and a link to the contact page which includes which product the enquiry is about. This code works

The code looks very messy to me and I think there are more <?php tags than are needed ?> and also I think it should be if and else not elseif.
Also I don't think the second stock check is necessary as it is an either or situation.
Could someone give me a hand as I keep breaking it and getting things in the wrong place.
i would just like to clean it up.


<!--bof stock announcement-->
<?php
if ((STOCK_CHECK == 'true')&&($product_info['products_quantity'] < 1)) { ?>
<b><font color='red'size='5'>Sorry We Have Sold Out</font></b><br>
<?php  echo '<b><a href="' . tep_href_link(FILENAME_CONTACT_US,'products_name=' .$product_info['products_name']) . '">' .tep_image_button('button_ask_a_question.gif',IMAGE_BUTTON_ASK_A_QUESTION) .'</a>'; ?>
<?php
}elseif ((STOCK_CHECK == 'true')&&($product_info['products_quantity'] > 0)) { ?>
<font color='green'size='4'><b>In Stock&nbsp;</b></font>
<?php
} ?>
<!--eof stock announcement-->


#2   MrPhil

MrPhil
  • Members
  • 4,149 posts

Posted 27 April 2012 - 03:00 PM

Yes, it can be cleaned up if all the switching in and out of PHP bugs you...
<!--bof stock announcement-->
<?php
if (STOCK_CHECK == 'true' && $product_info['products_quantity'] < 1) {
  echo "<b><font color='red'size='5'>Sorry We Have Sold Out</font></b><br>\n";
  echo '<b><a href="' . tep_href_link(FILENAME_CONTACT_US, 'products_name=' . $product_info['products_name']) . '">' . tep_image_button('button_ask_a_question.gif', IMAGE_BUTTON_ASK_A_QUESTION) . "</a>\n";
} elseif (STOCK_CHECK == 'true' && $product_info['products_quantity'] > 0) {
  echo "<font color='green'size='4'><b>In Stock&nbsp;</b></font>\n";
}
?>
<!--eof stock announcement-->

And please keep indentation when posting code (within [ code ] tags). "elseif" is a standard, valid PHP construct. If it bugs you, "else if" should be an acceptable replacement. Note that there is no "else" clause here -- coders should be careful to check that nothing happening, if none of the "ifs" fire, is acceptable. I also like to end HTML lines with \n so that the lines are not all run together on the browser source listing (requires use of " instead of ').

#3   burt

burt

    Code Monkey

  • Community Team
  • 7,764 posts

Posted 27 April 2012 - 10:30 PM

<!--bof stock announcement-->
<?php
if (STOCK_CHECK == 'true') {
  $message = '<font color="green" size="4"><b>In Stock&nbsp;</b></font>';
  if ($product_info['products_quantity'] <= 0) {
	$message = '<b><font color="red" size="5">Sorry We Have Sold Out</font></b><br>';
	$message .= '<b><a href="' . tep_href_link(FILENAME_CONTACT_US,'products_name=' .$product_info['products_name']) . '">' . tep_image_button('button_ask_a_question.gif', IMAGE_BUTTON_ASK_A_QUESTION) .'</a>';
  }
  echo $message;
}
?>
<!--eof stock announcement-->

Would also work similarly I think (untested).

Edited by burt, 27 April 2012 - 10:35 PM.

Dummies guide to designing osCommerce 2.3 Click Me

Or maybe a ready made theme for your shop ??

Warning: My posts may contain Horsemeat.

#4   MrPhil

MrPhil
  • Members
  • 4,149 posts

Posted 27 April 2012 - 10:37 PM

Change the test in @burt's code from > 0 to < 1.

#5   douglaswalker

douglaswalker
  • Members
  • 31 posts

Posted 28 April 2012 - 03:09 AM

Hi
Thank you very much for taking the time to answer I really appreciate it.
Both codes do the job well.
Is there any reason why you would use on over the other... or is it just personal preference.
Doug

#6   burt

burt

    Code Monkey

  • Community Team
  • 7,764 posts

Posted 28 April 2012 - 09:18 AM

I'd use the version i posted simply because it's the logical structure that I feel is easiest to understand, for the average person.

I do believe (though I have not tested) that a product with zero in stock would not show the "out of stock" message on your original code or Phils cleanup of it.  

Assuming that `0 stock` is an oversight on your part, then you do not need to test $product_info['products_quantity'] twice.  You certainly don't need to test STOCK_CHECK twice...
Dummies guide to designing osCommerce 2.3 Click Me

Or maybe a ready made theme for your shop ??

Warning: My posts may contain Horsemeat.

#7   douglaswalker

douglaswalker
  • Members
  • 31 posts

Posted 28 April 2012 - 01:21 PM

Hi Burt
thanks for that. I am using your code and tested with Phils <1 in place of <=0
It is all working well and  <1 does the job.
Less than one is 0 and below or am I missing something?
Once again many thanks
Doug :blink:

#8   MrPhil

MrPhil
  • Members
  • 4,149 posts

Posted 28 April 2012 - 03:47 PM

Assuming product_quantity == 0, both the original and my revision should work. 1 or more should be "in stock". < 0 probably won't happen, but should be treated the same as 0. I'm assuming that product_quantity is an integer amount.

@burt, please explain your contention that product_quantity of 0 won't work in the original code or my revision.

#9   burt

burt

    Code Monkey

  • Community Team
  • 7,764 posts

Posted 29 April 2012 - 11:56 AM

View PostMrPhil, on 28 April 2012 - 03:47 PM, said:

@burt, please explain your contention that product_quantity of 0 won't work in the original code or my revision.

No need - my mistake :D

"I do believe (though I have not tested) that a product with zero in stock would not show the "out of stock" message on your original code or Phils cleanup of it. "

That will teach me not to be reading multiple threads at once :-\
Dummies guide to designing osCommerce 2.3 Click Me

Or maybe a ready made theme for your shop ??

Warning: My posts may contain Horsemeat.