How can i improve this code [closed]

Closed. This question needs to be more focused. It is not currently accepting answers.

---

Want to improve this question? Update the question so it focuses on one problem only by editing this post.

Closed 5 years ago.

开发者_如何转开发 Improve this question

I have added this function in my user class.

• Is this function Place in the User Class is right.
• How should i handle the database query error in this function.
• Or I can Use any Database class for executing the queries that will helpful.
• What should it return to the call of the function.

Please Help. Thanks

    function adduser() {

        // Storing data in database
         $sql = "INSERT INTO users  (   alias,  firstname,  lastname,   PASSWORD,email  )
            VALUES  (   '$this->alias',     '$this->firstname',     '$this->lastname',  AES_ENCRYPT('$this->password','text'),'$this->email'    );";
        $result = mysql_query($sql);

        $this->userid=mysql_insert_id();

        //make profile card in cards table...
        $sql="INSERT INTO cards ( userid_from,  userid_to,  eventid,    gibid, 
            card_type,  message ,status,isDeck) VALUES  (   '$this->userid',    '$this->userid',    'eventid',  '$spaceid',     'V', '','A','Y' )"  ;   
        @mysql_query($sql);     
        $id_card=mysql_insert_id(); 

        $systemgibid=systemgibid();
        //make system gib card in cards table...
        $sql="INSERT INTO cards ( userid_from,  userid_to,  eventid,    gibid, 
            card_type,  message ,status,isDeck) VALUES  (   '',     '$this->userid',    'eventid',  '$systemgibid',     'A', '','A','N' )"  ;   
        @mysql_query($sql);     


        $this->firstname=$this->firstname."\'s Gib";
        //create gibs define in connection.php type D for default gib
        creategib($this->firstname,'D',$this->userid);


    }

---

The biggest thing you can do is protect your SQL by using prepared statements. Right now you are vulnerable to the classic SQL injection vulnerability.

More on prepared statements: http://php.net/manual/en/pdo.prepared-statements.php

If you do not want to use PDO, at the very least use mysql_real_escape_string

---

Is this function Place in the User Class is right. Yes

How should i handle the database query error in this function. Use try {} catch () {} , throw an exception if anything was wrong

Or I can Use any Database class for executing the queries that will helpful. Use PDO for db queryes

What should it return to the call of the function. boolean true on success , throw exception on failure log the exception and return false

---

Three things:

1. STOP, please STOP using '@' to suppress errors. Use exception handling.
2. Try to use PDO or escape the query parameters using mysql_real_escape_string
3. Let all the querying be done form a single class. It will keep your exception handling cases (As mentioned in point 1) in a single place.

---

HI USE try{}catch things ans set error message whenever you get. and than catch it in exception. and than display it all.