We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 33968
    • 863 Posts
    Hi all

    This is my first attempt at building a simple-ish web app using xPDO. It seems to be working fine, although there is plenty of code that I feel is not as efficient as it could be. I’ve been studying the docs, but at this stage I don’t sufficiently understand all of the methods to know which is the better approach.

    The snippet below is nagging at me in particular. If anyone can take a look and suggest where it could be improved I would be very grateful!

    I’ve placed explanations and my questions (1) and (2) in the code as it seemed easier to follow that way.

    Thanks!

    <?php // syntax highlighting
    
    // $shortlist is an associative array containing 'home' and 'lot' pairings
    // eg. array( [0] => array( ['homeid'] => 15 ['lotid'] => 226 ) )
    foreach ($shortlist as $pair) {
    
        // 'homes' are not related to 'lots' or 'estates'
        // (1) can I get the 'name' field without requiring the whole object?
        $home   = $modx->getObject('Home',$pair['homeid']);   
        $home   = $home->get('name');        
    
        // the objects below (Lot, Estate) *are* related  (Child, Parent respectively)
        // (2) is there a more efficient way to get the required fields?
        $lot    = $modx->getObject('Lot',$pair['lotid']);    
        $lotNo  = $lot->get('lot');
        $street = $lot->get('street');
        $estate = $lot->getOne('Estate');
        $estate = $estate->get('name');    	
        	
        $output .= ... etc ... ;  // output to placeholders
    }
    

    Note: I’m more interested in suggestions for question (2) for now smiley
      • 22873
      • 9 Posts
      To Q1: Are the Home objects really so huge? In the other case, any optimization here will hardly have a reasonable effect.

      To Q2: You can use newQuery with join here to spare one database request (but I am not sure here), but your code will be a bit less readable.

      But IMHO the most serious problem might be you have not a database table for the home-lot pairs. Using such table, you might be able to retrieve all the pairs with joined objects using only one database request (via getCollectionGraph).
        • 33968
        • 863 Posts
        Hi pavsyk, many thanks for the reply!

        As for Q1 - they are a reasonable size but not *huge*, so I guess it wouldn’t be a big problem. Just wondered if there can be a more efficient way to do it.

        Q2 - thanks, I will try that out and see if there is a performance difference.

        As for your suggestion regarding Home-Lot pairs, this is something I had wondered about. Homes and Lots are in no way related, but I need to list results containing one Home and One lot per result. It would be a Many-to-Many situation, with roughly 100 Home designs and potentially thousands of lots. The resulting intermediary table could have up to 500,000 rows!

        I don’t know a lot about how MySql works, but would this likely be slower than just making the two queries?
          • 22873
          • 9 Posts
          Hi lucas,

          IMHO, you definitely have a relationhsip between homes and lots now, although not in a table, but in your $shortlist array. Or is it a set of random homes against a random set of lots?

          Your approach in the code above leads to (IMHO) 3 database queries per each home-lot pair. The performance will depend on how many pairs you have and how many concurrent users will use it. E.g. if you will query the database with 5-10 pairs once a minute, it is waste of time to try optimize it. But in case of a couple of tens or hundreds pairs being queried each couple of seconds - you should definitely think about another approach. If it is not a typical many-to-many relationship, you can for example query the database once for a collection of homes and once for a collection of lots (possibly including the estates). And to pair them afterwards.

            • 33968
            • 863 Posts
            It’s more like random pairings of homes and lots, as selected and shortlisted by visitors as they browse the site. Almost any combination is possible (there are some size restrictions but not important here). So you’re right, there is a relationship... but it’s decided by the visitor and not something I can control.

            Certainly for now the code doesn’t need optimising for performance - this is more out of my own interest and from a best-practice point of view. The lot/estate pairs can be brought down to a single query as they are related. Is that an inner join? I’ll give that a try for now.

            Really appreciate your thoughts!
              • 33968
              • 863 Posts
              Happy to say that I’ve more than halved execution time of the above by:

              • moving the queries out of the foreach loop. I knew that was bad but until now I hadn’t figured out a way around it. Instead I’ve two getCollection calls before the loop, one for homes and one for lots. Results go into arrays that I use to lookup the shortlist pairings.
              • using an innerJoin on Lots and Estates to reduce the total number of queries to just two. Don’t think I can get below that.

              Much better smiley