2
\$\begingroup\$

I have written this file in PHP using MySQL. However, I have a lot of code that looks like it could be simplified. Can someone suggest a way to make this more efficient? i.e. I want to follow the D.R.Y method.

<?php
  require_once('database.php');
  try{

    $result = $conn->query('SELECT id FROM documentaries');

  }catch(Exception $e){
    echo $e->getMessage();
    die();
  }

  $docs = $result->FetchAll(PDO::FETCH_ASSOC);

?>

<!DOCTYPE html>
<html>
  <head>

      <meta charset="utf-8">
      <title>Tom Turner - Director of Photography</title>
      <link rel="stylesheet" href="css/normalize.css">
      <link href='https://fonts.googleapis.com/css?family=Changa+One|Open+Sans:400,400italic,700,700italic,800' rel='stylesheet' type='text/css'>
      <link rel="stylesheet" href="css/main.css">
      <meta name="viewport" content="width=device-width, initial-scale=1.0">
      <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.0/jquery.min.js"></script>
      <script type="text/javascript" src="js/jquery.mixitup.min.js"></script>
      <script type="text/javascript" src="js/main.js"></script>
      <script type="text/javascript" src="js/responsivemenu.js"></script>

  </head>
  <body>
    <header>
      <a href="index.php" id="logo">
        <h1>Tom Turner</h1>
        <h2>Director of Photography</h2>
      </a>
      <nav>

          <ul>
            <li><a href="index.php" >About</a></li>
            <li><a href="/portfolio.php" class="selected">Portfolio</a></li>
            <li><a href="/contact.php">Contact</a></li>
          </ul>

      </nav>
    </header>

    <div id="wrapper">

        <section>
            <div class="showreel-container">
                <iframe src="https://player.vimeo.com/video/148640837?title=0&byline=0&portrait=0" width="500" height="281" frameborder="0" webkitallowfullscreen mozallowfullscreen allowfullscreen></iframe>
            </div>
        </section>

        <section>

            <div id="controls" id="Controls">

                <button class="filter" data-filter="all">All</button>
                <button class="filter" data-filter=".documentaries">Documentary</button>
                <button class="filter" data-filter=".commercial">Commercial</button>
                <button class="filter" data-filter=".charity">Charity/NGO/Commisions</button>
                <button class="filter" data-filter=".music">Music</button>
                <button class="filter" data-filter=".drama">Drama</button>

            </div>



            <div id="Container" class="container">

            <div class="mix documentaries" data-myorder="1">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "1"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>
            <div class="mix documentaries" data-myorder="2">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "2"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-02" alt="Image two"/></a>';
                    }
                }
            ?>
            </div>
            <div class="mix documentaries" data-myorder="3">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "3"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>
            <div class="mix documentaries" data-myorder="4">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "4"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>
            <div class="mix documentaries" data-myorder="5">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "5"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>
            <div class="mix commercial" data-myorder="6">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "6"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>
            <div class="mix commercial" data-myorder="7">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "7"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>
            <div class="mix charity" data-myorder="8">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "8"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>

            <div class="mix charity" data-myorder="9">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "9"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>

            <div class="mix charity" data-myorder="10">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "10"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>

            <div class="mix charity" data-myorder="11">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "11"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>

            <div class="mix charity" data-myorder="12">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "12"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>

            <div class="mix music" data-myorder="13">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "13"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>

            <div class="mix music" data-myorder="14">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "14"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>

            <div class="mix drama" data-myorder="15">

            <?php
                foreach($docs as $doc){
                    if ($doc["id"] == "15"){
                        echo '<a href="item.php?id='.$doc["id"].'"><img src="img/numbers-01" alt="Image one"/></a>';
                    }
                }
            ?>
            </div>


            <div class="gap"></div>
            <div class="gap"></div>

        </div>


      </section>


    </div>

     <footer class="main-footer">



        <div id="footer-notes">
          <p>Tom Turner - Director of Photography</p>
          <p>&copy; Tom Turner - All Rights Reserved</p>
        </div>
       <div id="mayur">
          <p>&copy; 2015 Website by <a href="https//:www.mayurpande.com">Mayur Pande</a></p>

        </div>

        <div class="social-media">
          <ul>

              <li><a href="mailto:tom@allritesreversed.co.uk"><img src="img/mail_circle.png" alt="Email Logo" /></a></li>
              <li><a href="https://www.facebook.com/tom.turner.397501?fref=ts"><img src="img/fbcircle.png" alt="Facebook Logo" /></a></li>
              <li><a href="https://vimeo.com/user6107855"><img src="img/vimeo_circle.png" alt="Vimeo Logo" /></a></li>
              <li><a href="https://twitter.com/intent/tweet?screen_name=mayurpandeuk"><img src="img/twitter_circle.png" alt="Twitter Logo" /></a></li>

            </ul>
          </div>
      </footer>



  </body>
</html>
\$\endgroup\$
  • \$\begingroup\$ Quick error, you don't have a closing head tag \$\endgroup\$ – Canadian Luke Jan 31 '16 at 16:53
  • \$\begingroup\$ @CanadianLuke thanks for the help managed to notice this locally on my machine yesterday. Have amended the code above \$\endgroup\$ – mp252 Jan 31 '16 at 17:51
1
\$\begingroup\$

First of all separate your files. Means create layout for your page as below:

head.php

<head>
    <meta charset="utf-8">
    <title>Tom Turner - Director of Photography</title>
    <link rel="stylesheet" href="css/normalize.css">
    <link href='https://fonts.googleapis.com/css?family=Changa+One|Open+Sans:400,400italic,700,700italic,800' rel='stylesheet' type='text/css'>
    <link rel="stylesheet" href="css/main.css">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.0/jquery.min.js"></script>
    <script type="text/javascript" src="js/jquery.mixitup.min.js"></script>
    <script type="text/javascript" src="js/main.js"></script>
    <script type="text/javascript" src="js/responsivemenu.js"></script>

</head>

header.php

<header>
    <a href="index.php" id="logo">
        <h1>Tom Turner</h1>
        <h2>Director of Photography</h2>
    </a>
    <nav>
        <ul>
            <li><a href="index.php" >About</a></li>
            <li><a href="/portfolio.php" class="selected">Portfolio</a></li>
            <li><a href="/contact.php">Contact</a></li>
        </ul>
    </nav>
</header>

index.php

   require_once('database.php');
try {

    $result = $conn->query('SELECT id FROM documentaries');
} catch (Exception $e) {
    echo $e->getMessage();
    die();
}

$docs = $result->FetchAll(PDO::FETCH_ASSOC);

/**
 * Creating the array of with the name of classes and the keys are the 
 * id that are stored in the database.
 */
$classesArray = array(1 => 'documentaries',
    2 => 'documentaries',
    3 => 'documentaries',
    4 => 'documentaries',
    5 => 'documentaries',
    6 => 'commercial',
    7 => 'commercial',
    8 => 'charity',
    9 => 'charity',
    10 => 'charity',
    11 => 'charity',
    12 => 'charity',
    13 => 'music',
    14 => 'music',
    15 => 'drama',
);
?>

<!DOCTYPE html>
<html>
    <?php include './head.php'; ?>
    <body>
        <?php include './header.php'; ?>
        <div id="wrapper">
            <section>
                <div class="showreel-container">
                    <iframe src="https://player.vimeo.com/video/148640837?title=0&byline=0&portrait=0" width="500" height="281" frameborder="0" webkitallowfullscreen mozallowfullscreen allowfullscreen></iframe>
                </div>
            </section>
            <section>
                <div id="controls" id="Controls">
                    <button class="filter" data-filter="all">All</button>
                    <button class="filter" data-filter=".documentaries">Documentary</button>
                    <button class="filter" data-filter=".commercial">Commercial</button>
                    <button class="filter" data-filter=".charity">Charity/NGO/Commisions</button>
                    <button class="filter" data-filter=".music">Music</button>
                    <button class="filter" data-filter=".drama">Drama</button>
                </div>
                <div id="Container" class="container">
                    <?php
                    foreach ($docs as $doc) {
                        $docId = $doc['id'];
                        ?>
                        <div class="mix <?php echo $classesArray[$docId]; ?>" data-myorder="<?php echo $docId; ?>">
                            <a href="item.php?id=<?php echo $docId; ?>"><img src="img/numbers-0<?php echo $docId; ?>" alt="Image <?php echo $docId; ?>"></a>
                        </div>
                    <?php } ?>
                </div>

                <div class="gap"></div>
                <div class="gap"></div>
        </div>
    </section>
    <?php include './footer.php'; ?>
</body>
</html>

In above file we have created the array of classes assuming that these are the fixed classes you are using based on Id. When iterating over the $docs array we are putting class name after mix dynamically with the help of $classesArray and the id from the database.

footer.php

<footer class="main-footer">
    <div id="footer-notes">
        <p>Tom Turner - Director of Photography</p>
        <p>&copy; Tom Turner - All Rights Reserved</p>
    </div>
    <div id="mayur">
        <p>&copy; 2015 Website by <a href="https//:www.mayurpande.com">Mayur Pande</a></p>

    </div>
    <div class="social-media">
        <ul>

            <li><a href="mailto:tom@allritesreversed.co.uk"><img src="img/mail_circle.png" alt="Email Logo" /></a></li>
            <li><a href="https://www.facebook.com/tom.turner.397501?fref=ts"><img src="img/fbcircle.png" alt="Facebook Logo" /></a></li>
            <li><a href="https://vimeo.com/user6107855"><img src="img/vimeo_circle.png" alt="Vimeo Logo" /></a></li>
            <li><a href="https://twitter.com/intent/tweet?screen_name=mayurpandeuk"><img src="img/twitter_circle.png" alt="Twitter Logo" /></a></li>

        </ul>
    </div>
</footer>
\$\endgroup\$
0
\$\begingroup\$

I want to follow the D.R.Y method.

Sure you're right, especially in the current case!

Obviously the trick is to generate all the #Container div at once.
There are a lot of ways to do it, here is one. It takes place just after you got results from your query :

// build contents

foreach ($docs as $doc) {
  $docId = $doc['id'];
  ob_start();
?>
<div class="mix documentaries" data-myorder="<?php echo $docId; ?>">
  <a href="item.php?id=<?php echo $docId; ?>">
    <img src="img/numbers-01" alt="Image one"/>
  </a>
</div>
<?php
  $html[] = ob_get_clean();
}
?>

Let's comment some points:

  • I used ob_start() then ob_get_clean(), to improve readability: it allows writing HTML "naturally", with embedded PHP, rather than writing PHP with strings which are parts of HTML. In addition, it takes advantage of the editors highlighting capabilities.
  • each generated <div> is saved in the $html array, so further in the HTML code (in <div id="Container">) we can merely replace all your previous code by <?php echo implode("\n", $html); ?> ("\n" keeps the actual generated HTML more readable).

Now looking at your code a bit further I noticed that you wrote:

  • <img src="img/numbers-01" alt="Image one"/> for doc #1
  • then <img src="img/numbers-02" alt="Image two"/> for doc #2
  • and again <img src="img/numbers-01" alt="Image one"/> for doc #3 to the end

So it may come from a copy/paste error so it might mean that either:

  1. each doc #X should use <img src="img/numbers-X" alt="Image X"/>
  2. or all docs should use the same unique <img src="img/numbers-01" alt="Image one"/>
  3. or even like in your posted code (1, 2, 1, 1, 1...)

The code showed above was good for #3. Now for #1, the following statement:

<img src="img/numbers-01" alt="Image one"/>

should become:

<img src="img/numbers-<?php echo substr($docId + 100, 1); ?>"
alt="Image <?php echo $wordified[$docId]; ?>"/>

and we should previously define the needed strings:

$wordified = [
  'zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine',
  'ten', 'eleven', 'twelve', 'thirteen', 'forteen', 'fithting',
];

("zero" is here for convenience, while not used)

Then if the real need was #3 (that I don't believe), you'd have to define an match-list, something like $match = [0, 1, 2, 1, 1, 1...]; (in the precise current case), and use $match[$docId] instead of $docId in the image definition.

Here is the whole code (version #1 above, assuming there is an image for each doc):

require_once('database.php');

try {
  $result = $conn->query('SELECT id FROM documentaries');
} catch(Exception $e) {
  echo $e->getMessage();
  die();
}

$docs = $result->FetchAll(PDO::FETCH_ASSOC);

$wordified = [
  'zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine',
  'ten', 'eleven', 'twelve', 'thirteen', 'forteen', 'fithting',
];
// build contents
foreach ($docs as $doc) {
  $docId = $doc['id'];
  ob_start();
?>
<div class="mix documentaries" data-myorder="<?php echo $docId; ?>">
  <a href="item.php?id=<?php echo $docId; ?>">
    <img src="img/numbers-<?php echo substr($docId + 100, 1); ?>"
    alt="Image <?php echo $wordified[$docId]; ?>"/>
  </a>
</div>
<?php
  $html[] = ob_get_clean();
}
?>
<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title>Tom Turner - Director of Photography</title>
    <link rel="stylesheet" href="css/normalize.css">
    <link href='https://fonts.googleapis.com/css?family=Changa+One|Open+Sans:400,400italic,700,700italic,800' rel='stylesheet' type='text/css'>
    <link rel="stylesheet" href="css/main.css">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.0/jquery.min.js"></script>
    <script type="text/javascript" src="js/jquery.mixitup.min.js"></script>
    <script type="text/javascript" src="js/main.js"></script>
    <script type="text/javascript" src="js/responsivemenu.js"></script>
  </head>

  <body>
    <header>
      <a href="index.php" id="logo">
        <h1>Tom Turner</h1>
        <h2>Director of Photography</h2>
      </a>
      <nav>
        <ul>
          <li><a href="index.php" >About</a></li>
          <li><a href="/portfolio.php" class="selected">Portfolio</a></li>
          <li><a href="/contact.php">Contact</a></li>
        </ul>
      </nav>
    </header>

    <div id="wrapper">
      <section>
        <div class="showreel-container">
          <iframe src="https://player.vimeo.com/video/148640837?title=0&byline=0&portrait=0" width="500" height="281" frameborder="0" webkitallowfullscreen mozallowfullscreen allowfullscreen></iframe>
        </div>
      </section>

      <section>
        <div id="controls" id="Controls">
          <button class="filter" data-filter="all">All</button>
          <button class="filter" data-filter=".documentaries">Documentary</button>
          <button class="filter" data-filter=".commercial">Commercial</button>
          <button class="filter" data-filter=".charity">Charity/NGO/Commisions</button>
          <button class="filter" data-filter=".music">Music</button>
          <button class="filter" data-filter=".drama">Drama</button>
        </div>

        <div id="Container" class="container">
          <?php echo implode("\n", $html); ?>
          <div class="gap"></div>
          <div class="gap"></div>
        </div>
      </section>
    </div>

    <footer class="main-footer">
      <div id="footer-notes">
        <p>Tom Turner - Director of Photography</p>
        <p>&copy; Tom Turner - All Rights Reserved</p>
      </div>
     <div id="mayur">
        <p>&copy; 2015 Website by <a href="https//:www.mayurpande.com">Mayur Pande</a></p>
      </div>

      <div class="social-media">
        <ul>
          <li><a href="mailto:tom@allritesreversed.co.uk"><img src="img/mail_circle.png" alt="Email Logo" /></a></li>
          <li><a href="https://www.facebook.com/tom.turner.397501?fref=ts"><img src="img/fbcircle.png" alt="Facebook Logo" /></a></li>
          <li><a href="https://vimeo.com/user6107855"><img src="img/vimeo_circle.png" alt="Vimeo Logo" /></a></li>
          <li><a href="https://twitter.com/intent/tweet?screen_name=mayurpandeuk"><img src="img/twitter_circle.png" alt="Twitter Logo" /></a></li>
          </ul>
        </div>
    </footer>
  </body>
</html>
\$\endgroup\$
  • \$\begingroup\$ this code does not seem to work. I get an error within the html for some reason. It seems to look fine to me though so not sure where it has gone wrong. Also where you have made the <div> element I don't think this will work properly as this is a jquery plugin, and in order for the jquery filter to work for the portfolio it needs to have a div class="mix" but it also needs to have the category class to group it in-order for the filter to work likes this <div class="mix documentaries"> where the class documentaries could be music,drama, charity etc. Hope this make sense \$\endgroup\$ – mp252 Feb 1 '16 at 10:54
  • \$\begingroup\$ @mp252 How does it not work for you? An which error do you got? I just added a $docs sample to work without your db: it works successfully, i.e. echo implode("\n", $html); generates in <div id="Containers">the exact HTML code it replaced, especially the <div class="mix documentaries"> you mention. So I don't understand what you mean. \$\endgroup\$ – cFreed Feb 1 '16 at 12:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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