3

I'm a beginner at PHP and PDO.

The code below works, but it's very very slow. Is there any way to make it more efficient?

while() isn't that efficient but does the work. I have tried other solutions but nothing really works.

Any suggestions?

    public function register_new_member(){

        global $bcrypt; 
        global $populera;

        $query = $this->db->prepare("SELECT id, lid, firstname, surname FROM `members`");

        try{
            $query->execute();      
            while ($data = $query->fetch()){

            $id = $data['id'];
            $lid = $data['lid'];

            $username = $populera->create_username($data['firstname'],$data['surname']);    

            //Show data
            echo "<br>Username: ".$username;
            echo "<br>ID: ".$id;
            echo "<br>LID: ".$lid;      

            //Static password - change after login      
            $password = "password";

            //Bcrypt    
            $pass = $bcrypt->generateHash($password);

            //Show data
            echo "<br>Password: ".$pass;


            $query2  = $this->db->prepare("INSERT INTO `login` (`id`, `lid`, `user`, `password`) VALUES (?, ?, ?, ?) ");

            $query2->bindValue(1, $id);
            $query2->bindValue(2, $lid);
            $query2->bindValue(3, $username);
            $query2->bindValue(4, $pass);

            $query2->execute();
            }
            //Success? 
            echo "<br>Saved!";

            }catch(PDOException $e){
                die($e->getMessage());
            }
}
Freelancer
  • 836
  • 1
  • 14
  • 47
Per76
  • 184
  • 1
  • 1
  • 15

4 Answers4

1

I think the efficiency problem here is that you are running $query2->execute() for each row in the result of the first query. You could do a multiple insert to make it more efficient.

How to insert multiple rows in one query

Community
  • 1
  • 1
benjosantony
  • 537
  • 4
  • 13
1

You aren't clear about what you mean by very very slow.

You could try this query to populate your login table in bulk.

INSERT INTO login (id, lid, user)
SELECT id,
       lid,
       CONCAT(firstname, ' ', surname) AS user
  FROM members

That should update all the rows you need.

Finally, you'll have to do this from php to set the passwords. This will set all the null or blank passwords to your starting point password.

        //Bcrypt    
        $pass = $bcrypt->generateHash($password);
        $query2  = $this->db->prepare("UPDATE login SET password  ? WHERE password IS NULL OR LENGTH(password) = 0");
        $query2->bindValue(1, $pass);
        $query2->execute();
O. Jones
  • 103,626
  • 17
  • 118
  • 172
1

The hash function may be slow -- since you're setting the password the same for all users you could move this calculation out of the loop. (It should be slow because the hash is usually repeated for some number of "rounds" to make brute forcing hard.)

I can't tell from your code if you are using a random salt for each hash. If you are (and you should be), then this suggestion would leave you with many passwords hashed with the same salt. I'm not sure how risky this is given that you're setting them all to the same value anyway.

Personally I would try to come up with a solution where I didn't preset the passwords--e.g. generating a random token that I email each user so they can create a new password for themselves. (Something you might need anyway to let users reset forgotten passwords.)

mike__t
  • 979
  • 5
  • 6
1

I would think of couple optimizations for this:

  1. Prepare your insert outside of the while loop (there is no point re-prepare it and it is costly)
  2. Do all inserts within transaction,if there is massive amount of data, just commit them smaller chunks or use save points of mysql.
crusa
  • 11
  • 1