7
\$\begingroup\$

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:

\$\endgroup\$
  • \$\begingroup\$ it there a specific reason you don't know/don't want to hardcode the column names, but instead look them up each time? \$\endgroup\$ – tim Apr 3 '15 at 13:54
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ – JustCarty Apr 3 '15 at 13:56
2
\$\begingroup\$
  • added $colCount = count($colNames);
  • removed unset()
  • added grave accents to $table
  • replaced for with foreach
  • replaced MYSQLI_ASSOC with MYSQLI_NUM
  • removed $iii in DESCRIBE Loop
  • renamed $tableHead to $row
  • renamed $jjj to $y
  • renamed $iii to $x
  • renamed $retval to $results
  • removed $conn (only needed if accessing multiple databases)
  • renamed $tableContent to $xyTable
  • removed unnecessary vertical white space
  • added mysqli_free_result($result);
  • padded and typed variable declarations first

New code

    $x = 0;
    $colCount = 0;
    $row = array(); 
    $name = '               '; 
    $sql = "DESCRIBE `$table`               ";
    $colNames = array();
    $xyTable= array();
    $results = mysqli_query($sql);
    while ($row = mysqli_fetch_array($results, MYSQLI_NUM)){$colNames[] = $row[0];}
    $colCount = count($colNames);
    $sql = "SELECT * FROM $table";
    $results = mysqli_query($sql);
    while ($row = mysqli_fetch_array($results, MYSQLI_NUM)){
      foreach ($colNames as $x => $name){
        $xyTable[$x][$name] = $row[$x];
      }
    }
 mysqli_free_result($result);

$xyTable[$x][$name] is wasteful.
It would be better to use the column number.
In this case the for loop is better than the foreach

$colCount = count($colNames);
$x = 0;
for ($y = 0; $y < $colCount; $y++){
  $xyTable[$x][$y] = $row[$y];
  $x++;
}

Then when you need the column name use colNames[$y]


Only one query was necessary:

It would be better to remove the DESCRIBE Query and
use mysqli_fetch_fields() after the SELECT Query:

$sql = "SELECT * FROM $table";
$results = mysqli_query($sql);
while ($row = mysqli_fetch_fields($result)){$colNames[] = $row[0];}
while ($row = mysqli_fetch_array($retval, MYSQLI_NUM)){

I have not yet Bench marked while ($row = mysqli_fetch_array()){} against:

$array = mysqli_fetch_all();
foreach($array as $key => $val){}

I'm betting on "while fetch_array" over "fetch_all foreach"


I will address your question on the unset(). Which is now moot because it never should have existed at that point.

The short answer on removing the unset is the unset destroys the variable $iii if the unset are both in global scope. If this were an include called from a function and $iii was not declared locally to the function, then it will be locally destroyed but not globally destroyed.

No matter what, you should not have unset it. Especially as you are using it in the next routine.

How a variable is destroyed is a compiler decision. Sometimes the value is set to NULL, most times the allocated memory for that variable is freed to be reallocated for other use.

Even if you were not going to use it again you still should not have used unset(). Because it is an integer uses minimum memory space, the CPU cycles use to destroy it would be a waste of resources. A well designed, compiler would ignore your unset.

Because PHP is an interpreter and not compiler it does not have the luxury of doing iterative optimization parsing as a compiler. The question that comes to mind; Is your first $iii and the $iii used after unset() using the same physical memory location? Probably not. But when you remove the unset it will undoubtedly remain the same.

Your $retval (most PHP programmers use $result) should be "unset" if it is considerable in size. When a variable is large enough to consume RAM that may be needed before the end of script execution, it should be unset, except in the case of mysqli_query Result instead use mysqli_free_result($result);.

When and When Not to use unset

When and When Not to use unset() is not always an easy decision.

Static variables are where the data type and amount of memory required remains constant throughout its use. This also means the amount of physical memory allocated and it location (physical RAM address) remain constant. Where in physical memory it is stored depends largely on scope.

There are languages that require the number of elements and data type of the array's elements, and maximum string size be fixed throughout their use. Many, if not most, languages do not allow type conversion of variables. There are very valid reasons for doing it this way.

PHP is very flexible with arrays and variables allowing the executing code to change the the size and data type of variables. There are performance penalties for doing it this way.

The other factor is scope.

Why scope is important goes beyond knowing where the variable's value is valid. It also has an major impact on performance and memory usage, particularly the read and write.

Allocation of memory, static vs. dynamic, and scope go hand in hand.

In general terms, global scope the variable is declared and its value can be used and its value changed anywhere in the application. Module variables are defined to be used in a subset of procedures and functions. And local scope that are only valid with in user defined functions / procedures and their subroutines.

The way current Intel processors work is they have separate memory pages for instructions and data in the cache memory. Cache is where you want your variables to reside. The only better storage location would be in one of the processor's registers, but that is a decision for the compiler. Compilers may store a copy of the for loop variable ($jjj) in a register and then write its value back to it memory location at the end of the loop. (reason for loop is faster than while in PHP)

Use local variables rather than global where possible. If you keep your local variable storage requirements small and well organized you stand a good chance of all your data residing in L1 cache. Same applies to the number of machine instructions required for your executing code.

Declare all variables, by data type and size, before the code. Using unset() and declaring new variables within the code will fragment memory.

As an example of good memory organization and allocation, I declared $sql as a 32 byte string. and $name as 16 byte string. If I thought a field name would be longer than 16 characters I would have declared it as 32 byte.
On the extreme side but efficient. Going even further, I could have used the results from mysqli_fetch_fields() and initialized $row to the type and size or the columns using $col instead, $col = mysqli_fetch_fields()
.

It is a good idea for programmers to understand the basics of microprocessor design. Especially the instruction pipeline, cache, and microcode execution. It's all in Intel's 64 and IA-32 Architectures Optimization Reference Manual:

Intel's 64 and IA-32 Architectures Optimization Reference Manual

I recommend Chapter 3 and all "User/Source Coding" Rules.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you very much for such a detailed answer, I am very grateful. As I said, I know very little and am self taught so I pick up bad practices or don't quite know what I am doing; so thank you very much for clearing up some things for me. \$\endgroup\$ – JustCarty Apr 6 '15 at 10:04
4
\$\begingroup\$
  • 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.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – JustCarty Apr 3 '15 at 14:12
  • \$\begingroup\$ 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!) \$\endgroup\$ – RubberDuck Apr 3 '15 at 14:13
2
\$\begingroup\$

$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.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

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.

| improve this answer | |
\$\endgroup\$

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.