Jump to content

Having trouble sorting prices in a foreach loop


ZionBludd
 Share

Recommended Posts

Hi Team, I am having trouble sorting prices in my foreach loop and am wondering is becasuse my price field is a text field and not a number field.

My code is 

<?php
$item_title = $item->title;
$items = $pages->find ("template=Listing, limit=15, sort=title");
foreach ($items as $itemūüėě?>
<tr>
<td>{item.brand}</td>
<td>{item.model}</td>
<td>
<?php $item_title = $item->title;
$store_price = $pages->find ("template=PriceSpy, sort=price, title=$item_title, limit=1") ;
foreach ($store_price as $price) :?>
 
{price.price}
 
<?php endforeach ;?>
 
 
</td>
<td>
<?php $item_title = $item->title;
$store_price = $pages->find ("template=PriceSpy, sort=-price, title=$item_title, limit=1") ;
foreach ($store_price as $price) :?>
 
 
{price.price}
 
<?php endforeach ;?>
 
 
</td>
</tr>
<?php endforeach ;?>

The offending page is this one: https://www.vapeprices.co.nz/price-guide/. Whereas you can see the Low and High columns are just not making sense

I would really appreciate any help.

Link to comment
Share on other sites

2 minutes ago, Gideon So said:

Hi @Matt Cohen

There should be your price field name not price in the selector.

$store_price = $pages->find ("template=PriceSpy, sort=-{your_price_field_name}, title=$item_title, limit=1") ;

Gideon

Hi Gideon, the price field that it's been sorted by is the price field name. 

Link to comment
Share on other sites

Hi Matt,

Your code looks like it should work okay so not sure why it isn't. But a few general observations...

Making the connection between pages with the Listing template and pages with the PriceSpy template via only the title is not very robust. What if one of these pages needed to change title? You'd then have to do maintenance on all related pages or else the connection would break. I think it would be better to make the connection via Page Reference fields or possibly a parent-child relationship.

Using a text field for prices is not ideal, and I think you should exclude the currency symbol from the field (I'm assuming the $ sign is in the field because I can't see it being added separately in your code). Better to use a decimal field type and prepend the currency symbol in your template. Then there would be fewer complications with sorting like you're experiencing now. 

It looks like you're using a templating language (is that Smarty?), but there is business logic in the template also which sort of defeats the most common reason for choosing to use a template language separate from PHP. If you're not bothered about enforcing a strict MVC pattern it would be simpler just to stick to plain PHP in your template files.

Edited by Robin S
Fixed double-post
  • Like 4
Link to comment
Share on other sites

Perhaps you need to change this

$pages->find("template=PriceSpy, sort=price, title=$item_title, limit=1") 

to this:

$pages->find("template=PriceSpy, sort=price, title=$item_title")->first()

 

  • Like 1
Link to comment
Share on other sites

6 hours ago, Robin S said:

Hi Matt,

Your code looks like it should work okay so not sure why it isn't. But a few general observations...

Making the connection between pages with the Listing template and pages with the PriceSpy template via only the title is not very robust. What if one of these pages needed to change title? You'd then have to do maintenance on all related pages or else the connection would break. I think it would be better to make the connection via Page Reference fields or possibly a parent-child relationship.

Using a text field for prices is not ideal, and I think you should exclude the currency symbol from the field (I'm assuming the $ sign is in the field because I can't see it being added separately in your code). Better to use a decimal field type and prepend the currency symbol in your template. Then there would be fewer complications with sorting like you're experiencing now. 

It looks like you're using a templating language (is that Smarty?), but there is business logic in the template also which sort of defeats the most common reason for choosing to use a template language separate from PHP. If you're not bothered about enforcing a strict MVC pattern it would be simpler just to stick to plain PHP in your template files.

Thanks heaps for your suggestions, Robin. I am looking to improve the structure over the weekend & will take these on board

 

5 hours ago, tpr said:

Perhaps you need to change this


$pages->find("template=PriceSpy, sort=price, title=$item_title, limit=1") 

to this:


$pages->find("template=PriceSpy, sort=price, title=$item_title")->first()

 

Thanks tpr, Will look into this! 

  • Like 1
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
√ó
√ó
  • Create New...