Jump to content

Is there a problem doing this this way, or is there a better way?


Recommended Posts

I have this little bit of queries (About 160 right now)


<?php
$queries = array();
$q = "select FacID From `".F_TABLE."` Order by FacID";
$r = mysql_query($q) or die(mysql_error());
$ids = array();
while($row = mysql_fetch_array($r)){
$ids[] = $row['FacID'];
}
$query = array();
foreach($ids as $id){
if($_POST[$id.'_active'] == "on"){
$queries[$id][0] = "Update `".F_TABLE."` Set ";
$queries[$id][1] = "Active = '1'";
}
elseif($_POST[$id.'_active'] != "on"){
$queries[$id][0] = "Update `".F_TABLE."` Set ";
$queries[$id][1] = "Active = '0'";
}
if($_POST[$id.'_coaching'] == "on" && $_POST[$id.'_counseling'] != "on" && $_POST[$id.'_facilitaing'] != "on"){
$queries[$id][2] = ", Type = '1'";
}
elseif($_POST[$id.'_coaching'] != "on" && $_POST[$id.'_counseling'] == "on" && $_POST[$id.'_facilitaing'] != "on"){
$queries[$id][2] = ", Type = '2'";
}
elseif($_POST[$id.'_coaching'] != "on" && $_POST[$id.'_counseling'] != "on" && $_POST[$id.'_facilitaing'] == "on"){
$queries[$id][2] = ", Type = '3'";
}
elseif($_POST[$id.'_coaching'] == "on" && $_POST[$id.'_counseling'] != "on" && $_POST[$id.'_facilitaing'] == "on"){
$queries[$id][2] = ", Type = '4'";
}
elseif($_POST[$id.'_coaching'] == "on" && $_POST[$id.'_counseling'] == "on" && $_POST[$id.'_facilitaing'] != "on"){
$queries[$id][2] = ", Type = '5'";
}
elseif($_POST[$id.'_coaching'] != "on" && $_POST[$id.'_counseling'] == "on" && $_POST[$id.'_facilitaing'] == "on"){
$queries[$id][5] = ", Type = '6'";
}
elseif($_POST[$id.'_coaching'] == "on" && $_POST[$id.'_counseling'] == "on" && $_POST[$id.'_facilitaing'] == "on"){
$queries[$id][5] = ", Type = '7'";
}
if(isset($queries[$id][0])){
$queries[$id][3] = " Where FacID = '".$id."'";
$query[$id] = implode(" ",$queries[$id]);
}
}
foreach($query as $value){
mysql_query($value) or die(mysql_error());
}
?>
[/Code]

it basically sets a few things and then runs the 160 queries, it is running very quickly now (less than a second to complete), but should i worry if I say had 1000 users.  I can partition it, but is that nessecary?  I know the ugly if statements could be cleaned up, but I do not know if I need to add another field to that for now I will leave it as it is

 

If I am reading all of that right, then yes there is a better way.

 

You have a lot of extra looping and logic that is not needed.

 

You first build an array from a db query and then create another loop to go through the array. All you need is to just go through the query and skip the interim array.

 

Also, in your logic you are building the queries in array pieces and concatenating them together at the end. No need, just concatenate as you go.

 

Instead of all the if/else statements I would either save each value in seperate columns or use a bitwise value: i.e. the first value is either 0 or 1, the 2nd value is either 0 or 2, the 3rd value is either 0 or 4. I have left that out of the code below:

 

<?php
$q = "select FacID From `".F_TABLE."` Order by FacID";
$r = mysql_query($q) or die(mysql_error());

while ($row = mysql_fetch_array($r)) {

    $id = $row['FacID'];

    $queries[$id] = "Update `".F_TABLE."` Set ";
    $queries[$id] .= ($_POST[$id.'_active'] == 'on') ? "Active = '1'" : "Active = '0'";

    if($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] != 'on' && $_POST[$id.'_facilitaing'] != 'on'){
        $queries[$id] .= ", Type = '1'";
    }
    elseif($_POST[$id.'_coaching'] != 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] != 'on'){
        $queries[$id] .= ", Type = '2'";
    }
    elseif($_POST[$id.'_coaching'] != 'on' && $_POST[$id.'_counseling'] != 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '3'";
    }
    elseif($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] != 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '4'";
    }
    elseif($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] != 'on'){
        $queries[$id] .= ", Type = '5'";
    }
    elseif($_POST[$id.'_coaching'] != 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '6'";
    }
    elseif($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '7'";
    }

    $queries[$id] .= " Where FacID = '".$id."'";

}

foreach($queries as $query){
mysql_query($query) or die(mysql_error());
}
?>

Yeah i realize I could get rid of making the array of ids and just do it in the while loop, but as for time saving that is minimal, i was more concerned about the number of queries, but I don't think there is a way around updating so many rows as you can only update a row at a time.

I realize this isn't going to solve your problem, but it will make your code look cleaner,

 

First insert this function into your script:

<?php
function test_on($x, $y, $z) {
$x_on = 1;
$y_on = 2;
$z_on = 4;
$tot = 0;
$tot += ($x == 'on')?$x_on:0;
$tot += ($y == 'on')?$y_on:0;
$tot += ($z == 'on')?$z_on:0;
return($tot);
}?>

Then you can replace these lines:

<?php
    if($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] != 'on' && $_POST[$id.'_facilitaing'] != 'on'){
        $queries[$id] .= ", Type = '1'";
    }
    elseif($_POST[$id.'_coaching'] != 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] != 'on'){
        $queries[$id] .= ", Type = '2'";
    }
    elseif($_POST[$id.'_coaching'] != 'on' && $_POST[$id.'_counseling'] != 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '3'";
    }
    elseif($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] != 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '4'";
    }
    elseif($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] != 'on'){
        $queries[$id] .= ", Type = '5'";
    }
    elseif($_POST[$id.'_coaching'] != 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '6'";
    }
    elseif($_POST[$id.'_coaching'] == 'on' && $_POST[$id.'_counseling'] == 'on' && $_POST[$id.'_facilitaing'] == 'on'){
        $queries[$id] .= ", Type = '7'";
    }
?>

with

<?php
$queries[$id] .= ", Type = '" . test_on($_POST[$id.'_coaching'], $_POST[$id.'_counseling'], $_POST[$id.'_facilitaing']) . "'";
?>

 

Ken

I have toyed with the idea of a math function since right now i have

1 = coach

2 = counselor

3 = facilitator

 

but I realized that if they are a coach and counselor that the sum is 3 which is facilitators when it should be 4.  Also I may want to include additional types in addition to what I already have so a function right now would not work until I solved it.  I'm trying to hopefully create this specific solution to a more generic one I can reuse in any user backend system.  Odds are I will probably add sql backing to it at some point where the text tags are part of a table, and a generalized equation can be generated.  That or I should maybe think about instead of making all these different types that are varients of others into instead make a table

Types    Ranking

1                Coach

2          facilitator

3            Counselor

etc

 

Then I simply would have a select for the type in the display and it just draw it off that table.  Make sense  I.e

 

<?php
$q = "select Rankings, Types from `Rankings` Order by Types";
$r = mysql_query($q) or die(mysql_error());
while($row = mysql_fetch_array($r)){
$ranks[$row['Rankings']] = $row['Type'];
}

//Then from post comes the Rankings or the Type and I can find the other.
?>

 

Did you see my comment about using bit operators? You just need a number with three bits. 000 all options are false, 111, all options are true.

 

Coach is the first bit with a value of 0 or 1

Counselor is the 2nd bit with a value of 0 or 2

Facilitator is the 3rd bit with a value of 0 or 4.

 

then you can determine the type using simple math and you will not have a value that can mean multiple things:

 

<?php

$type_val = 0;
$type_val .= ($_POST[$id.'_coaching']=='on') ? 1 : 0;
$type_val .= ($_POST[$id.'_counseling']=='on') ? 2 : 0;
$type_val .= ($_POST[$id.'_facilitaing']=='on') ? 4 : 0;
queries[$id] .= ", Type = '$type_val'";
?>

 

Also, one way to prevent having to run some of the queries would be to utilize some javascript. I assume there are a fixed number of fields for each ID. You could also create a hidden field for each set of fields to track whether any of the fields are changed (e.g. "id#_changed"). Then simply create an onchange event for each field that will change the value of that field to "true".

 

however, you will want to ensure that the form will work accordingly if the user has JS disabled. I would populate those fields with the default value of "true". Then have an onload event to change them to "false". That way if JS is disabled all the fields will be flagged as changed (since you cannot track if the fields are changed with JS. However, if JS is enabled (in most cases) you can simply ignore processing the fields for IDs where the changed value is false.

@mjdamato

If you're trying to create bitwise OR operations, why are you using .= operator?  Won't that end up with type having values like:

14

12

24

124

etc?

 

@cooldude

You have inputs on your form that look like this:

  <input type="checkbox" name="{$ID}_active" />
  <input type="checkbox" name="{$ID}_coaching" />
  etc..

Which leads to a post array that looks like this:

Array(
  [5_active] => 'on',
  [5_coaching] => 'on',
  etc.
)

 

I find it is much easier to create my inputs in this format:

  <input type="checkbox" name="info[{$ID}][active]" />
  <input type="checkbox" name="info[{$ID}][coaching]" />
  etc..

Which leads to a post array that looks like this:

Array(
  [info] => Array(
    [5] = Array(
      [active] => 'on',
      [coaching] => 'on'
    ),
    [7] = Array(
      [active] => 'on'
    )
  )
)

 

What I'm getting at is whenever I build forms, I try and coerce the inputs into a format that makes form processing take the least amount of effort and thought on my part.  The first step in doing so is organizing the way the data is given to me.  I'll post back in a bit with my attempt at modifying your code as it is right now.

Assuming I followed your code correctly, I made the following truth table:

coaching | counseling | facilitating | TYPE
---------+------------+--------------+-----
   T     |     T      |      T       |  7  
   T     |     T      |      F       |  5  
   T     |     F      |      T       |  4  
   T     |     F      |      F       |  1  
   F     |     T      |      T       |  6  
   F     |     T      |      F       |  2  
   F     |     F      |      T       |  3  
   F     |     F      |      F       |  #  

 

From the looks of the table, it doesn't look like you can solve this with bit-wise operations.  I'll prove it to you:

#1: The following row suggests that counseling would represent the second bit: 010

   F     |     T      |      F       |  2  

#2: The following row suggests that coaching is the first bit: 001

   T     |     F      |      F       |  1  

#3: If we ORed coaching and counseling when both were true, we should get: 011 -> 3

#4: In fact, we get: 101 -> 5 (CONTRADICTION)

   T     |     T      |      F       |  5  

 

K, I'll have a solution in a few.  :D

This code cuts down the number of queries run, not the number of loops made.  The way I accomplish this is grouping all the FacIDs into categories:

Active, Inactive, and then those that correspond to the truth table.

 

Then I run a single query for each category (Active, Inactive, TTT, TTF, TFT, etc) for a total of 10 queries.  You should also note that your original code never handled the situation of 'FFF', so my code just tries to set the field to NULL in the database.  You could modify the switch statement to skip over that particular key if you wanted.

 

Anywho, here it is.  It's untested so there could be parse errors.  In addition, it doesn't execute the final queries but just echos them out for verification.  I suggest you make sure it's operating correctly before letting it loose on your data, or back your data up first.

 

<?php
$TTable = Array( // Define our truth table
  'TTT' => 7,
  'TTF' => 5,
  'TFT' => 4,
  'TFF' => 1,
  'FTT' => 6,
  'FTF' => 2,
  'FFT' => 3,
  'FFF' => 'NULL'
);
$SQL_IDs = Array(   // Group our IDs into arrays to limit the amount of queries
  'Active' => Array(), 'Inactive' => Array()
);
foreach($TTable as $key => $val){
  $SQL_IDs[$key] = Array(); // create an array for each truth table key
}

$q = "select FacID From `".F_TABLE."` Order by FacID";
$r = mysql_query($q) or die(mysql_error());
while($row = mysql_fetch_assoc($r)){   // for each ID in the table
  $id = $row['FacID'];                 // Short hand variable
  // Generate short hand variables for the check boxes
  $chkActive = isset($_POST[$id.'_active']) &&
               $_POST[$id.'_active'] == 'on' ? true : false;
  $chkCoaching = isset($_POST[$id.'_coaching']) &&
                 $_POST[$id.'_coaching'] == 'on' ? 'T' : 'F';
  $chkCounseling = isset($_POST[$id.'_counseling']) &&
                   $_POST[$id.'_counseling'] == 'on' ? 'T' : 'F';
  $chkFacilitating = isset($_POST[$id.'_facilitating']) &&
                     $_POST[$id.'_facilitating'] == 'on' ? 'T' : 'F';

  // At this point, chkActive is equal to true or false
  // And each of chkCoaching, chkCounseling, and chkFacilitating are
  // equal to 'T' or 'F'

  // Determine where to insert this ID into our SQL_IDs array
  if($chkActive){
    $SQL_IDs['Active'][] = $id;
  }else{
    $SQL_IDs['Inactive'][] = $id;
  }
  $SQL_IDs[$chkCoaching.$chkCounseling.$chkFacilitating][] = $id;
}

// Run our queries
foreach($SQL_IDs as $key => $ids){
  switch($key){
    case 'Active':
      $sql = "Update `".F_TABLE."` Set Active = '1' WHERE FacID IN ("
           . implode(", ", $ids) . ")";
      break;
    case 'Inactive':
      $sql = "Update `".F_TABLE."` Set Active = '0' WHERE FacID IN ("
           . implode(", ", $ids) . ")";
      break;
    default:
      $sql = "Update `".F_TABLE."` Set Type=" . $TTable[$key] . " WHERE FacID "
           . "IN (" . implode(', ', $ids) . ")";
      break;
  }
  // DEBUG -- print the sql for verification
  echo '<pre style="text-align: left;">' . print_r($sql, true) . '</pre>';
  //mysql_query($sql) or die("ERROR: " . mysql_error());
}
?>

 

(EDIT) I also noticed in your code a couple of your conditions you were setting $queries[$id][5] instead of $queries[$id][2].  I assume that was either an error on your part OR you were trying to add an 'AND' condition to your WHERE part.  If you were trying to add an AND condition then my code is going to do the wrong thing; also, your syntax for the AND condition would be wrong anyways, which is why I assumed you made a typo.

 

Also, I said my code doesn't decrease the amount of loops, I misspoke.  It does decrease the amount of loops in the original code, but it still has to loop over every entry in the database.  When I said it doesn't decrease the amount of loops I was referring to the fact that it still has to loop over all the database entries; that's the best I could achieve with the information given.

@mjdamato

If you're trying to create bitwise OR operations, why are you using .= operator?  Won't that end up with type having values like:

14

12

24

124

etc?

 

You are correct I made a mistake. It should be +=

 

From the looks of the table, it doesn't look like you can solve this with bit-wise operations.

 

Well of course it wouldn't work with the current type values he is using now. My suggestion implied that he would change his type values accordingly.

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.