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 6 down vote favorite

I am a beginner to PHP programming. I wanted to make a 2D array that would store the values of a table into the array. Below is the Frankenstein code I created (every time I wanted to achieve a task I researched a solution, so this is a bunch of solutions plus my own doing put together).

I would be really grateful if anyone could offer any improvements to my code. One thing I am slightly unsure of is the use of the unset function. I understand that it 'destroys' a variable to free up memory(?), but I am not entirely sure where/ if to use it.

<?php
$sql = "DESCRIBE $table"; // get a list of all fields and datatypes (ignore the latter)
$retval = mysqli_query($conn, $sql);
$iii = 0; // counter variable
while ($tableHead = mysqli_fetch_array($retval, MYSQLI_ASSOC)) 
{
    $colNames[$iii] = $tableHead["Field"]; // store each column name in array
    $iii++;
}
unset($iii);

$sql = "SELECT * FROM $table"; // get a list of all fields and datatypes (ignore the latter)
$retval = mysqli_query($conn, $sql);
$iii = 0; // counter variable
while ($row = mysqli_fetch_array($retval, MYSQLI_ASSOC)) 
{
    // create a 2D array, $iii for x and $jjj for y
    for ($jjj = 0; $jjj < $colCount; $jjj++)
    {
        $tableContent[$iii][$colNames[$jjj]] = $row[$colNames[$jjj]];
    }
    $iii++; // move onto next row
}
unset($iii, $jjj);
?>

EDIT: I have attached some pictures of what I have created. The quick report just reads the ID and returns any ID that is not found in the database or in the CSV. The full report is not yet developed but will check in more depth by checking each field one at a time and reporting the differences to the user.

The CSV looks like this:

1,a
2,b
3,c
4,d
5,e
6,f
7,g
8,h
9,i

The front page:

The result:

share|improve this question
    
it there a specific reason you don't know/don't want to hardcode the column names, but instead look them up each time? –  tim yesterday
1  
I am creating a table lookup program that will take a CSV file of data, and check it against any table; therefore the field names may vary. –  Carty yesterday

3 Answers 3

up vote 4 down vote

  • This comment lies.

    $sql = "SELECT * FROM $table"; // get a list of all fields and datatypes (ignore the latter)

    You copy pasted the whole line and then neglected to update/remove the comment.

  • It's traditional to use a simple i for loop counter variables. I see no reason to use a triple iii or jjj. However, in this case it makes much more sense to use more verbose (and hence, descriptive) names like $row and $col.

share|improve this answer
    
Whoops, note to self: pay more attention ;) Thank you for the suggestions; I'll change those now. What about the unset? I am unsure where/ if to use those in my code. Thank you. –  Carty yesterday
    
I've no idea. I've never written a line of php in my life. Sorry I can't help with that. You might want to call out that you're unsure of that in your question. (Oh! and you're welcome!) –  RubberDuck yesterday
up vote 2 down vote

$colNames and $tableContent are not defined at an earlier stage, thus the scope is unclear. This is hard(er) for maintaining the software.

Error handling is missing. mysqli_query returns false if the query has failed (eg. database is unavailable). What value(s) are you going to return when the code is unable to execute the queries?

Also I think unset is unnecessary. You're saving less than 32 bytes of memory here.

share|improve this answer
up vote 0 down vote

A row with column names is what mysqli_fetch_array using MYSQLI_ASSOC already returns - no need to use separate query for these names. Also note $array[] notation inside while loop - equivalent for array_push() function - adding new rows will result in 2D array you need. Try this: 

$sql = "SELECT * FROM $table";
$res = mysqli_query($conn, $sql);
while ($row = mysqli_fetch_array($res, MYSQLI_ASSOC)) {
    $tableContent[] = $row;
}

While loop might be replaced with:

$tableContent = mysqli_fetch_all($res, MYSQLI_ASSOC);

but it's problematic with large result sets (like dumping entire table) and further processing it would require a loop anyway.

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.