0
\$\begingroup\$

I am a self-taught front-end developer, trying to land a job, the PHP and MySQL are quite new to me, since I only watched and did some code through Traversy Media courses.

This is a test project, where I need to build a product list page that pulls data from a database and presents them in tables/boxes. The entries are presented with checkboxes, so you can delete them and add other products from an add product page that I am currently trying to make...

It's functioning as it is, however I am curious whether this can be written more profesionally.

<?php
$pdo = new PDO('mysql:host=localhost;port=3306;dbname=test', 'bork', '123456');
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$statement = $pdo->prepare('SELECT * FROM skandi Where id=1');
$statementTwo = $pdo->prepare('SELECT * FROM skandi Where id=2');
$statementThree = $pdo->prepare('SELECT * FROM skandi Where id=3');
$statement->execute();
$statementTwo->execute();
$statementThree->execute();
$products = $statement->fetchAll(PDO::FETCH_ASSOC);
$product = $statementTwo->fetchAll(PDO::FETCH_ASSOC);
$productt = $statementThree->fetchAll(PDO::FETCH_ASSOC);
?>
<!DOCTYPE html>
<html lang="en">
<head>
 <meta charset="UTF-8" />
 <meta http-equiv="X-UA-Compatible" content="IE=edge" />
 <meta name="viewport" content="width=device-width, initial-scale=1.0" />
 <title>Document</title>
 <link rel="stylesheet" href="style/normalize.css" />
 <link rel="stylesheet" href="style/style.css" />
</head>
<body>
 <header>
 <h2>Product List</h2>
 <nav>
 <button class="add-btn" id="addBtn">ADD</button>
 <button class="mass-btn" id="delete-product-btn" onclick="removeCheckedCheckboxes()">MASS DELETE</button>
 </nav>
 </header>
 <section class="product-list-wrapper">
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($products as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Size']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($products as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Size']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($products as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Size']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($products as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Size']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($product as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Weight']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($product as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Weight']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($product as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Weight']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($product as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Weight']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($productt as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Dimension']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($productt as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Dimension']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($productt as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Dimension']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($productt as $i => $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Dimension']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 </section>
 <footer>
 <h2>Scandiweb Test Assignement</h2>
 </footer>
 <script src="js/main.js"></script>
</body>
</html>
Mast ♦
13.8k12 gold badges56 silver badges127 bronze badges
asked Jul 5, 2022 at 10:44
\$\endgroup\$
6
  • 1
    \$\begingroup\$ Welcome to Code Review! We need to know what the code is intended to achieve. To help reviewers give you better answers, please add sufficient context to your question, including a title that summarises the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. The title needs an edit to simply state the task, rather than your concerns about the code. \$\endgroup\$ Commented Jul 5, 2022 at 12:51
  • \$\begingroup\$ Whoever sent you here from stack overflow should have sent you to our help pages starting with this one How do I ask a good question?. \$\endgroup\$ Commented Jul 5, 2022 at 12:58
  • \$\begingroup\$ Currently there is one vote to close this question here on Code Review, if you add the details requested this is a great question and will probably get you one or more answers. \$\endgroup\$ Commented Jul 5, 2022 at 15:46
  • \$\begingroup\$ To help you rewrite the code I think it would be better to have more of your code than less. The reason is that it sounds like you're repeating code, and by removing this we cannot see what is being repeated. \$\endgroup\$ Commented Jul 5, 2022 at 19:29
  • 1
    \$\begingroup\$ I have no idea why $products is identically iterated three times. I can't review what I don't understand. On CodeReview, it is a good idea to be patient with awarding the green tick so that your question appears to invite more reviews. \$\endgroup\$ Commented Jul 6, 2022 at 8:43

2 Answers 2

1
\$\begingroup\$

Since the example code shows for each of the 3 product results 4 identical entries, the first step would be to replace the 4 div with a for loop.

And because the structure of the 3 product tables are almost identical – they differ only in one key (namely 'Size', 'Weight', and 'Dimension') –, the second step would be to simplify that into another, outer for loop.

...
// after querying the database:
$all = [ $products, $product, $productt ];
...
// replacement for all 12 divs:
<section class="product-list-wrapper">
 <?php foreach ($all as $entry) : ?>
 <?php for ($j = 0; $j < 4; $j++) : ?>
 <div class="div-box">
 <input type="checkbox" class="delete-checkbox" />
 <table>
 <tbody>
 <?php foreach ($entry as $item) : ?>
 <tr class="content">
 <td><?php echo $item['SKU']; ?> </td>
 <td><?php echo $item['Name']; ?> </td>
 <td><?php echo $item['Price']; ?> </td>
 <td><?php echo $item['Size'] ?? $item['Weight'] ?? $item['Dimension']; ?> </td>
 </tr>
 <?php endforeach; ?>
 </tbody>
 </table>
 </div>
 <?php endfor; ?>
 <?php endforeach; ?>
</section>
...

Edit: Based on @mickmackusa's comment below this could be a way to reduce the three database trips to one:

$statement = $pdo->prepare('SELECT * FROM skandi WHERE id = IN(1, 2, 3)');
$statement->execute();
$products = $statement->fetchAll(PDO::FETCH_ASSOC);
$all = [
 array_filter($products, fn($item) => $item['id'] === 1),
 array_filter($products, fn($item) => $item['id'] === 2),
 array_filter($products, fn($item) => $item['id'] === 3)
];

Depending on the count of rows returned from the database, this might be faster or slower. My assumption would be that for very large amount of rows it would be faster to query three times, because database engines excel at filtering, while for smaller amounts of rows, one query with three array_filter calls would be faster.

answered Jul 6, 2022 at 8:15
\$\endgroup\$
6
  • \$\begingroup\$ Is your review endorsing the three separate trips to the database or are you intentionally omitting that aspect from your answer? \$\endgroup\$ Commented Jul 6, 2022 at 8:27
  • \$\begingroup\$ Thanks for the assistance lukas.j, works like a charm, and also a great learning moment for me, to better understand php... \$\endgroup\$ Commented Jul 6, 2022 at 8:30
  • \$\begingroup\$ @mickmackusa I omitted this intentionally for the following reason: The OP was about writing the code – which apparently works correctly – more compact and there is not much gain in that aspect by working over the queries (which additionally are not totally sensical to me: it seems that there are repeating ids, or each result would contain only one row each, and then the innermost for loops could be dropped). \$\endgroup\$ Commented Jul 6, 2022 at 8:34
  • \$\begingroup\$ I would use a CASE in the SELECT, but I've voted to close as unclear so I cannot post a review. \$\endgroup\$ Commented Jul 6, 2022 at 13:32
  • \$\begingroup\$ Hey, so I tried the edited code, however I am getting: Syntax error or access violation: 1064... However I forgot to mention that I need to be able to delete the tables(which I managed to do with JS code) and adding new ones through another page, which I am having trouble with... \$\endgroup\$ Commented Jul 6, 2022 at 15:06
0
\$\begingroup\$

Regarding PDO part, the code could be much simpler, thanks to a specific PDO fetch mode,

$statement = $pdo->query('SELECT id, skandi.* FROM skandi WHERE id = IN(1, 2, 3)');
$all = $statement->fetchAll(PDO::FETCH_GROUP|PDO::FETCH_ASSOC);
answered Jul 7, 2022 at 9:06
\$\endgroup\$
8
  • \$\begingroup\$ This is great, however Initially I didn't see that I am required to add a form to a product.php to upload data to database which would then take me to index.php where the new data should appear... I've done that, however this method won't work if new ID's appear... \$\endgroup\$ Commented Jul 7, 2022 at 12:33
  • \$\begingroup\$ if you need all ids then why you're using WHERE at all? \$\endgroup\$ Commented Jul 7, 2022 at 12:34
  • \$\begingroup\$ I am very new to PHP and MYSQL and learning things on the go, since I am low on time to finish the task... What is your suggestion? \$\endgroup\$ Commented Jul 7, 2022 at 12:44
  • \$\begingroup\$ if you need all ids then just remove the where clause \$\endgroup\$ Commented Jul 7, 2022 at 12:50
  • 2
    \$\begingroup\$ You closed the question, yet answered it. Why? \$\endgroup\$ Commented Jul 7, 2022 at 15:56

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.