views:

212

answers:

5

Hiya,

Can anyone suggest tips or alterations to make this code cleaner and faster? This was the only way I could think of doing it on a Friday evening, but I'm sure there must be a more efficient way of doing it...

I know regexs aren't efficient but I can't honestly see how else I can do this, especially if the Postcode data can be anything from:

e1 2be e1ebe e10ebe e10 ebe ex1 ebe ex1ebe

and so on...

Thanks a lot for any coding tips, H

$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Amma Gawd! Someone ate our database!');
    mysql_select_db($dbname);
    $result = mysql_query("SELECT * FROM `Consumer`
         WHERE left(`Postcode`,2) = 'E' 
         OR left(`Postcode`,1) = 'N'
         OR left(`Postcode`,1) = 'W'");
    while($row = mysql_fetch_array($result))  { 
        $email =  $row['Email'];
        if (preg_match("/^[Ee]{1}[0-9]{2}/",$row['Postcode'])) {
         mysql_query("UPDATE `Consumer` SET `CONYES` = '1' WHERE `Email` = '$email'") or die ("Bugger");
         $counter = $counter +1;
        } elseif (preg_match("/^[Nn]{1}[0-9]{2}/",$row['Postcode'])) {
         mysql_query("UPDATE `Consumer` SET `CONYES` = '1' WHERE `Email` = '$email'") or die ("Bugger");
         $counter = $counter +1;  
        } elseif (preg_match("/^[Ww]{1}[0-9]{2}/",$row['Postcode'])) {
         mysql_query("UPDATE `Consumer` SET `CONYES` = '1' WHERE `Email` = '$email'") or die ("Bugger");
         $counter = $counter +1; 
        } 
    }

    $result1 = mysql_query("SELECT * FROM `Consumer`
         WHERE left(`postcode`,2) = 'BR' 
         OR left(`postcode`,2) = 'CR'
         OR left(`postcode`,2) = 'EC'
         OR left(`postcode`,2) = 'EN'
         OR left(`postcode`,2) = 'KT'
         OR left(`postcode`,2) = 'NW'
         OR left(`postcode`,2) = 'RM'
         OR left(`postcode`,2) = 'SE'
         OR left(`postcode`,2) = 'SM'
         OR left(`postcode`,2) = 'SW'
         OR left(`postcode`,2) = 'TW'
         OR left(`postcode`,2) = 'WC'
         OR left(`postcode`,2) = 'BD'
         OR left(`postcode`,2) = 'HG'
         OR left(`postcode`,2) = 'LS'
         OR left(`postcode`,2) = 'WF'
         OR left(`postcode`,2) = 'YO'
         OR left(`postcode`,2) = 'HD'
         OR left(`postcode`,2) = 'HX'");
    while($row1 = mysql_fetch_array($result1))  {   
        $email =  $row1['Email'];
        mysql_query("UPDATE `Consumer` SET `CONYES` = '1' WHERE `Email` = '$email'") or die ("Bugger");
        $counter = $counter +1; 
    }
    echo $counter;
    mysql_close($conn);
+2  A: 

One thing that might not be more efficent, but will look cleaner, you can use the IN MySQL operator:

SELECT * FROM `Consumer` WHERE left(`postcode`,2) IN ('BR', 'CR', 'EC', 'EN', 'KT', 'NW', 'RM'.....
gnarf
+6  A: 

You posted it as a PHP question, but I think the most efficient way is to do it all in one SQL query and get the database to do the work. You can use the keyword 'RLIKE' to get the database to perform regular expression matching. You should read up on the syntax to get exactly what you want, but just to start you off, you want something like this:

UPDATE `Consumer` SET `CONYES` = '1'
    WHERE `Postcode` RLIKE '[EeNnWwBbMm][0-9]{2}'
    OR LEFT(`postcode`,2) IN ('BR', 'CR', 'EC', 'EN', 'KT', 'NW', 'RM'.....

The result is the number of rows changed, which can be assigned directly to $counter.

Mark Byers
A: 

You could try matching the complement, and set CONYES = '1' on everything that does't match. Maybe that's an easier determination, like:

select * from Consumer where left(postcode, 2) <> 'XX'

or (psuedocode, I'm not a Perl guy):

if (!preg_match(complementRegexString, $row['Postcode'])
    mysql_query("UPDATE `Consumer` SET `CONYES` = '1' WHERE `Email` = '$email'") or die ("Bugger");
emptyset
A: 

You don’t need to update the database every time you encountered a match. It suffices if you just do it once:

$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Amma Gawd! Someone ate our database!');
mysql_select_db($dbname);
$result = mysql_query("
    SELECT `Postcode`, `Email`
    FROM `Consumer`
    WHERE LEFT(`Postcode`,1) IN ('E', 'N', 'W'");
$counter = 0;
while ($row = mysql_fetch_array($result)) {
    if (preg_match("/^[ENB][0-9]{2}/i",$row['Postcode'])) {
        if (!$counter) {
            $email =  $row['Email'];
            mysql_query("UPDATE `Consumer` SET `CONYES` = '1' WHERE `Email` = '$email'") or die ("Bugger");
        }
        ++$counter;
    }
}

$result = mysql_query("
    SELECT `Postcode`, `Email`
    FROM `Consumer`
    WHERE LEFT(`Postcode`, 2) IN ('BR', 'CR', 'EC', 'EN', 'KT', 'NW', 'RM', 'SE', 'SM', 'SW', 'TW', 'WC', 'BD', 'HG', 'LS', 'WF', 'YO', 'HD', 'HX')");
while ($row = mysql_fetch_array($result)) {
    if (!$counter) {
        $email = $row['Email'];
        mysql_query("UPDATE `Consumer` SET `CONYES` = '1' WHERE `Email` = '$email'") or die ("Bugger");
    }
    ++$counter;
}
echo $counter;
mysql_close($conn);

Here the database is only updated if there was no update yet (if $counter == 0 is true). If $counter has some other value than 0, use a different variable name.

You should also select only that columns you really need, in this case probably Postcode and Email.

Gumbo
+2  A: 

The sample code looks equivalent to the single query:

UPDATE `Consumer` SET `CONYES` = 1
   WHERE  Email IS NOT NULL 
     AND Postcode RLIKE '^([NEW][0-9]{2}|B[DR]|CR|E[CN]|H[DGX]|KT|LS|[NT]W|RM|S[EMW]|W[CF]|YO)'

The RE is less readable than the "IN" operator, but might be more performant. There might be a more suitable, more permissive and more correct regexp; the above was picked because it's equivalent to what's in the sample. The only other thing you need to do is get the number of affected rows, which is easy to do using PDO (which you should be using instead of the old MySQL driver, anyway):

try {
    $db = new PDO("mysql:host=$dbhost,dbname=$dbname", $dbuser, $dbpass);
    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $count = $db->exec("UPDATE `Consumer` SET `CONYES` = 1
       WHERE  Email IS NOT NULL 
         AND Postcode RLIKE '^([NEW][0-9]{2}|B[DR]|CR|E[CN]|H[DGX]|KT|LS|[NT]W|RM|S[EMW]|W[CF]|YO)'"
    );
    echo $count;
} catch (PDOException $exc) {
    // handle exception as you will
    error_log($exc);
    echo "I had an internal error. It's been logged, and we'll look into it.";
}
outis