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:[email protected]"><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>
asked Jan 30, 2016 at 15:27
\$\endgroup\$
2
  • \$\begingroup\$ Quick error, you don't have a closing head tag \$\endgroup\$ Commented Jan 31, 2016 at 16:53
  • \$\begingroup\$ @CanadianLuke thanks for the help managed to notice this locally on my machine yesterday. Have amended the code above \$\endgroup\$ Commented Jan 31, 2016 at 17:51

2 Answers 2

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:[email protected]"><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>
answered Feb 5, 2016 at 10:36
\$\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:[email protected]"><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>
answered Jan 31, 2016 at 2:22
\$\endgroup\$
2
  • \$\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\$ Commented Feb 1, 2016 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\$ Commented Feb 1, 2016 at 12:48

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.