current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite
1

I'm trying to see if there is a loop I can use in order to pull off this code a little more elegantly. I was previously using a foreach($row as $cell) with $row = mysqli_fetch_row($result) but because of that I couldn't access the data from the 'user' column in order to check it against the session variables in the if statment. Here's the code:

$sql = "SELECT * FROM tbl_name ORDER BY subject, description LIMIT {$startpoint},{$limit}";
$result = $mysqli->query($sql);
$num_rows = mysqli_num_rows($result);

if($num_rows>0){
$field_num = $mysqli->field_count;
echo "<h1>HERE ARE SOME RESULTS:</h1>";
echo "<table border='0'>";    

while($row = mysqli_fetch_array($result))
{
    echo"<tr>";
    echo"<td>".$row['subject']."</td>";
    echo"<td>".$row['description']."</td>";
    echo"<td>".$row['user']."</td>";
    if($row['user'] == $_SESSION['firstname']." ".$_SESSION['lastname']){
    echo"<td>You can delete this</td>";
    }
    else{
    echo"<td>Code didn't work</td>";
    }
    echo "</tr>\n";
}
mysqli_free_result($result);
}
else{
echo 'There are no results!';
}
?>

Is there a loop I can run that can also allow me to access data from each row so I don't have to print out a new line of code for each column but keep the if statement in tact? Thanks!

share|improve this question
    
You have not mentioned the structure of $result. You must be able to iterate the result set using foreach as you already tried before. Inside loop make sure you are pointing to data using correct index. –  Kinjal Nov 5 '12 at 7:44
    
Ah sorry, I've added more code. $result is a query from a specific MySQL table. –  MxmastaMills Nov 5 '12 at 14:25
    
What you've mentioned is exactly what I'm trying to do but I suppose I'm confusing on the syntax of that. That's the idea I have in my head but am not exactly sure how it is pulled off. –  MxmastaMills Nov 5 '12 at 14:27

3 Answers 3

up vote 2 down vote accepted

Saving lines of code in search of elegance is not always (in fact hardly ever in my experience) a worthwhile exercise if it comes at the expense of clarity and maintainability. Your current code is better than any solution that iterates over table columns because it is clear and easy to change. For example, combining the data of two columns into one or changing column order are both trivial.

You might consider moving this display logic to a template. Here is an example of a twig template:

{% if results %}
<h1>HERE ARE SOME RESULTS</h1>
<table>
    {% for row in results %}
    <tr>
        <td>{{ row.subject }}</td>
        <td>{{ row.description }}</td>
        {% if row.user == user %}
        <td>You can delete this</td>
        {% else %}
        <td>Code didn't work</td>
        {% endif %}
    </tr>
    {% endfor %}
</table>
{% else %}
<p>No results</p>
{% endif %}

The PHP becomes very simple:

$bindings = array(
    'results' => $mysqli->query('SELECT...'),
    'user' => $_SESSION['firstname'] . ' ' . $_SESSION['lastname']
);
$twig->render('template.html', $bindings);
share|improve this answer
    
No please, don't use a template engine. PHP is a template engine. –  Jefffrey Feb 14 '13 at 14:45
up vote 0 down vote

I would compare $user_id instead of the user's full name. What if you have two John Smiths?

If you use this to fetch your array:

$row = mysqli_fetch_array($result, MYSQLI_ASSOC); // Get row as associative array

then for each row, you can do this:

echo '<tr>';
foreach ($row as $cell_key => $cell_value) 
{
    echo '<td>'; // Single quotes because you don't need to eval the string

    if ($cell_key == 'user') 
    {
      if ($cell_value == $_SESSION['firstname'] . ' ' . $_SESSION['lastname'])
      {
          echo 'You can delete this';
      }
      else
      {
          echo 'Code didn\'t work';
      }
    } 
    else 
    {
        echo $cell_value;
    }

    echo '</td>'; // Single quotes because you don't need to eval the string

}

echo '<tr/>'; // Again single quoted, etc.

// And if you want to refer to a *specific* row here, you can just $row['name'] for example, yeah? That var is still available.

If the order of the columns isn't to your taste, don't SELECT *, select specific column names in the order you want them.

share|improve this answer
up vote 0 down vote

Mysqli_* functions or PDO

I would recommend you start using PDO, here is a good tutorial for it. If you insist on using mysqli_* the correct way is using foreach($row as $cell) as you already tried. Dunno why it didn't work for you then.

Sample PDO code

$host   = 'localhost';
$dbname = 'database1';
$user   = 'user1';
$pass   = 'password1';

try {
  # MySQL with PDO_MYSQL
  $dbh = new PDO("mysql:host=$host;dbname=$dbname", $user, $pass);
catch(PDOException $e) {
    echo $e->getMessage();
}

$sth = $dbh->query('SELECT * FROM tbl_name ORDER BY subject, description LIMIT :startpoint, :limit');
$sth->bindParam(':startpoint', $startpoint);
$sth->bindParam(':limit', $limit);

$sth->setFetchMode(PDO::FETCH_ASSOC);
while($row = $sth->fetch()) {
    echo $row['subject'] . "<br />";
    echo $row['description'] . "<br />";
    echo $row['user'] . "<br />";
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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