5
\$\begingroup\$

Okay so as I learn more about PHP and really strive to improve my code I have a few questions about a current setup of mine and how to approach it in the best way.

I think I have all the tools necessary to get this done the right way, but I am having trouble putting them together. I am also trying to separate my code better.

Scenario: the script in question is cabins.php which basically pulls information from the db to populate the page with cabin information based on the cabin id $_GET variable. So the user clicks on a cabin, then the page is populated with that cabins information. There are four sub menus to choose from for each cabin so I am accessing this information using a switch statement, but I am having trouble finding the best way to run my queries.

My data is structured that most of the cabin information is held in 'cabin_content' but I also have 'cabin_large_images' and 'cabin_small_images' which each hold images that are tied to the cabin_content table through cabin_id.

Each sub section requires different information from the cabin_content table, but each sub section ALWAYS requires all the images from cabin_large_images that match the cabin id.

So each section is like this: -we must query cabin_content and get the specific information we need -we must query cabin_large_images and get all the images with the cabin_id.

One section requires all of the above plus we have to query cabin_small_images and get all the images from that table with matching cabin id.

So right now I have it set up like this, which I know needs improvement...obviously that is why I am here...Pseudo code:

if(isset($_GET['cabin')){

 //run the query to get cabin_large_images because we know we need them regardless of the section

   if(isset($_GET['type'])){
      $type = $_GET['type'];

       switch ($type){

       case 'a':

//run the query on cabin_content for the specific info we need in this situation

       break;

       case 'b':

//run the query on cabin_content for the specific info we need in this situation

       break;

       case 'c':

//run the query on cabin_content for the specific info we need in this situation
//run an additional query on cabin_small_images

       break;


      }

 }




//populate content to view

Okay so obviously what I don't like about this is we are running as many as three queries per page load. I tried using inner joins, but I just could never access what I needed from the query properly:

SELECT cabin_name, peak_week, max_occupancy, image 
FROM cabin_content  
INNER JOIN cabin_images  
  ON cabin_content.cabin_id = cabin_images.cabin_id 
WHERE cabin_content.cabin_id = '$cabin'";

The issue here is that we can end up with multiple images from $row['images'] which we then need to iterate through in the view. Now, we complicate the situation even more when for case 'c' we also need to query cabin_small_images:

SELECT cabin_name, peak_week, max_occupancy, image, image_small 
FROM cabin_content  
INNER JOIN cabin_images  
  ON cabin_content.cabin_id = cabin_images.cabin_id 
INNER JOIN cabin_small_images 
  ON cabin_images.cabin_id = cabin_small_images.cabin_id 
WHERE cabin_content.cabin_id = '$cabin'";

So, from that I need all the regular content and I need to iterate through 'image' and 'small_images' and I just could never get it right.

Alright so in closing, if the way I have it structured is good, then fine, but I have a feeling the best way to do this is with the joins, in which case I need help accessing all the information from the query. Or I need your other conceptual suggestions on the best way to do this!

\$\endgroup\$
5
  • \$\begingroup\$ Bobby Tables strikes again. :) (fix that $cabin, imagine someone enters '); DROP TABLE cabin_content; --!) \$\endgroup\$
    – user36
    Dec 6, 2011 at 22:35
  • \$\begingroup\$ Try using LEFT JOINS instead of inner joins \$\endgroup\$
    – HLGEM
    Dec 6, 2011 at 23:00
  • 1
    \$\begingroup\$ I wasn't sure, thanks for checking. @WTP, yep, need to fix that, thanks. Just a matter of using mysqli_real_escape_string correct? \$\endgroup\$
    – absentx
    Dec 6, 2011 at 23:36
  • \$\begingroup\$ @absentx mysqli_real_escape_string would work, but you should take a look at PDO and prepared statements. Cleaner code, database agnostic and a lot of other goodies... \$\endgroup\$
    – yannis
    Dec 7, 2011 at 10:20
  • \$\begingroup\$ Yeah I really need to do that, PDO for my next project absolutely. \$\endgroup\$
    – absentx
    Dec 7, 2011 at 17:45

2 Answers 2

3
\$\begingroup\$

There is nothing wrong with using multiple queries. Three simple queries versus a single, convoluted monster query. Guess which option will cause you the least problems long term?

You get the extra overhead of parsing three queries, but, its about the same amount of work for the DBMS to get the data in both cases.

Also I think you should consider holding your images in separate files and just hold the file names in the database. You can then use HTML "<image>" tags to reference these. This will make the images "cache"able on the client side, and, will move the work of retrieving the images to the web server, which is much more efficient and will allow for a degree of parallelism when loading the users page.

\$\endgroup\$
1
  • \$\begingroup\$ I appreciate the feedback, I was just never sure about like you said, three small queries or one convoluted monster! I store the file names in the db then access them like this: <img src="../images/cabins/<?php echo $row['image']; ?>" width="524" height="276" alt="neato"> \$\endgroup\$
    – absentx
    Dec 7, 2011 at 2:09
0
\$\begingroup\$

I'm not 100% sure what you are asking.. & personally i see nothing wrong with using multiple queries. But i think what you are looking for is UNION

SELECT 
    cc.cabin_name
    ,cc.peak_week
    ,cc.max_occupancy
    ,'Large' as ImageType
    ,ci.image 
FROM cabin_content cc
INNER JOIN cabin_images ci ON cc.cabin_id = ci.cabin_id 
WHERE cc.cabin_id = '$cabin';
Union
SELECT 
    cc.cabin_name
    ,cc.peak_week
    ,cc.max_occupancy
    ,'Small' as ImageType
    ,csi.image_small
FROM cabin_content cc
INNER JOIN cabin_small_images csi ON cc.cabin_id = csi.cabin_id
WHERE cabin_content.cabin_id = '$cabin';
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.