-1
\$\begingroup\$

I have as recently as last week picked php back up to code in. My experience with it has always been quite limited, but as I am now making a simple application for my job (for internal use) I am stumbling upon some small difficulties (or questions rather).

<?php include_once 'config.php'; include_once 'browsercheck.php'; ?>
<html lang="nl-BE">
<head>
<title><?php echo $title; ?></title>
<link rel="stylesheet" href="style.css">
</head>
<body>
<div class="contentcontainer">
<?php

$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
die("Connection failed: " . $conn->connect_error);
}

$sql = "SELECT maat_id, maat_categorie, maat_nummer, maat_maatregel FROM stic_maat WHERE (maat_categorie = '1') ORDER BY maat_nummer ASC";
$result = $conn->query($sql);

$sql2 = "SELECT cat_id, cat_afkorting, cat_naam FROM stic_cat WHERE (cat_afkorting = 'ALG')";
$result2 = $conn->query($sql2);

$sql3 = "SELECT cat_id, cat_afkorting, cat_naam FROM stic_cat WHERE (cat_afkorting = 'NED')";
$result3 = $conn->query($sql3);

$sql4 = "SELECT maat_id, maat_categorie, maat_nummer, maat_maatregel FROM stic_maat WHERE (maat_categorie = 'NED') ORDER BY maat_nummer ASC";
$result4 = $conn->query($sql4);

?>
<h1 class="paginatitel">WebStic - Overzicht maatregelen</h1>
<table class="tabeloverview">
<th class="titeloverview" colspan="2">
<?php
if ($result2->num_rows > 0) {
// output data of each row
while($row2 = $result2->fetch_assoc()) {
echo '' . $row2["cat_afkorting"]. ' - ' . $row2["cat_naam"]. '';
}
} else {
echo 'Geen titel gevonden';
}
?>
</th>
<tr>
<td class="hoofdingoverview1">Nr.</td>
<td class="hoofdingoverview2">Maatregel</td>
</tr>
<?php   

if ($result->num_rows > 0) {
// output data of each row
while($row = $result->fetch_assoc()) {
echo '<tr><td class="inhoudoverview">' . $row["maat_nummer"]. '</td><td class="inhoudoverview">' . $row["maat_maatregel"]. '</td></tr>';
}
} else {
echo '<tr><td colspan="2" class="inhoudoverview">Geen maatregelen gevonden</td></tr>';
}

?>
<th class="titeloverview" colspan="2">
<?php
if ($result3->num_rows > 0) {
// output data of each row
while($row3 = $result3->fetch_assoc()) {
echo '' . $row3["cat_afkorting"]. ' - ' . $row3["cat_naam"]. '';
}
} else {
echo 'Geen titel gevonden';
}
?>
</th>
<tr>
<td class="hoofdingoverview1">Nr.</td>
<td class="hoofdingoverview2">Maatregel</td>
</tr>
<?php   

if ($result4->num_rows > 0) {
// output data of each row
while($row4 = $result4->fetch_assoc()) {
echo '<tr><td class="inhoudoverview">' . $row4["maat_nummer"]. '</td><td class="inhoudoverview">' . $row4["maat_maatregel"]. '</td></tr>';
}
} else {
echo '<tr><td colspan="2" class="inhoudoverview">Geen maatregelen gevonden</td></tr>';
}



?>
</table>

<p>
<a href="index.php">Terug naar algemeen beheer</a>
<p>
<div class="copyright">
<?php echo $copyright; ?>
</div>
<?php $conn->close(); ?>
</div>
</body>
</html>

There are four queries in my code: two to select content from table A and two to select content from table B. I then use the retrieved data to populate an html table, which does what it needs to.

There are currently two entries (categories) in table A. For each of these entries I need to add two queries: one to retrieve an exact row from table A and one to retrieve several rows where the category matches the one that was retrieved earlier.

The amount of entries in table A will rise to four, but in the future might rise to up to eight entries. This means I have to manually add two queries for each entry and duplicate the code to populate the html table.

The question (and I am not asking for someone to code this for me!): is it possible to retrieve all data once and then, using a while/if, parse all entries in a manner where the category name is parsed once and under each category name parse all entires related to that category?

Note: it is important that all categories and entries are parsed on the same page.

I am trying to avoid having to manually adjust the code each time a category is added.

edit: I have managed to fix it myself by taking a better look at te current code and making use of the already used while's and if's. Here's the adjusted code:

<?php

$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
die("Connection failed: " . $conn->connect_error);
}

$sql = "SELECT cat_id, cat_afkorting, cat_naam FROM stic_cat ORDER by cat_id ASC";
$result = $conn->query($sql);

?>
<h1 class="paginatitel">WebStic - Overzicht maatregelen</h1>
<table class="tabeloverview">

<?php
if ($result->num_rows > 0) {
// output data of each row
while($row = $result->fetch_assoc()) {
echo '<th class="titeloverview" colspan="2">' . $row["cat_afkorting"]. ' - ' . $row["cat_naam"]. '</th>';
echo '<tr><td class="hoofdingoverview1">Nr.</td><td class="hoofdingoverview2">Maatregel</td></tr>';

$catid = $row["cat_id"];

$sql2 = 'SELECT maat_id, maat_categorie, maat_nummer, maat_maatregel FROM stic_maat WHERE maat_categorie = "'.$catid.'" ORDER BY maat_nummer ASC';
$result2 = $conn->query($sql2);

if ($result2->num_rows > 0) {
// output data of eachS row
while($row2 = $result2->fetch_assoc()) {
echo '<tr><td class="inhoudoverview">' . $row2["maat_nummer"]. '</td><td class="inhoudoverview">' . $row2["maat_maatregel"]. '</td></tr>';
}
} else {
echo '<tr><td colspan="2" class="inhoudoverview">Geen maatregelen gevonden</td></tr>';
}               
}
} else {
echo '<th class="titeloverview" colspan="2">Geen titel gevonden</th>';
}
?>
</table>


<p>
<a href="index.php">Terug naar algemeen beheer</a>
<p>
<div class="copyright">
<?php echo $copyright; ?>
</div>
<?php $conn->close(); ?>

Thanks for the offered help.

\$\endgroup\$
  • \$\begingroup\$ Hi, welcome to CodeReview. Hope you get some great answers to your question. \$\endgroup\$ – Abbas Jan 30 '15 at 11:11
  • \$\begingroup\$ Could you show a 2nd page where you do the same, but this time for a different category? Right now there's no duplication to review and you're kinda asking for "gimme the codes". If you added a second page, you'd have duplication on your hands and it'd be on-topic to ask how to improve your code to not have this duplication. Additionally, it would help us to understand what is duplicate here; perhaps your notions that you have to rename all the variables are wrong. \$\endgroup\$ – Pimgd Jan 30 '15 at 11:12
  • \$\begingroup\$ Please state only the code's purpose in the title. \$\endgroup\$ – Jamal Jan 30 '15 at 13:50
1
\$\begingroup\$

I'm not a php guru, to be fair it's been a while that I even touched php.

I however believe that you need to work with GET or POST methods to select the category that you want to show, instead of statically picking it in your query.

It's however important to keep the data sanitized, for avoiding mysql injections.

Approach


A simple approach/suggestion I could give you is to use the following: Add at the beginning:

$category = $_GET["category"]

then at the query part, instead of having to worry about the sanitizing, just use a switch or if statement

if($category == 'ALG'){
   $sqlcat = "SELECT cat_id, cat_afkorting, cat_naam FROM stic_cat WHERE (cat_afkorting = 'ALG')";
}else if($category == 'RANDOM'){
   $sqlcat = "SELECT cat_id, cat_afkorting, cat_naam FROM stic_cat WHERE (cat_afkorting = 'ALG')";
}
$catresult = $conn->query($sqlcat);

I believe that you try to select the items based on the id of the category, correct me if I'm wrong. For that you should use the returned category result from the previous query.

    $row = $catresult->fetch_assoc();
    $catid =  $row["cat_id"];
    $sql = "SELECT maat_id, maat_categorie, maat_nummer, maat_maatregel FROM stic_maat WHERE (maat_categorie = "+$catid+") ORDER BY maat_nummer ASC";
    $result = $conn->query($sql);

Note that there can be some small syntactical errors in my sample. Still it should show the main idea.

Now let's say you normally access the page b going to website.com/list.php, you instead refer them to website.com/list.php?category=ALG The advantage here lies in the fact that you do not have to create a new php file for each category.

I hope this is more or less what you needed.

PS: welcome to Code Review ;)

\$\endgroup\$
  • \$\begingroup\$ You could probably get away with the more readable switch/case for this, also, don't concatenate variables in SQL queries, use prepared statements instead. \$\endgroup\$ – Madara Uchiha Feb 2 '15 at 8:52

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.