Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

There's a part in my page where you can add, edit or delete school details, and it wasn't supposed to save entries to the database until the user hits Enter, so I save all data in an object first.

My object looks like this:

schooldetails : {
  "1": {
    "school_ctr":"schoolctr_1",
    "school":"Fiat Lux Academe",
    "course":"",
    "qualification":"High School",
    "date_grad":"06/04/2008",
    "notes":"*graduated with honors",
    "deleted":0
  },
  "2": {
    "school_ctr":"schoolctr_2",
    "school":"College of St. Benilde",
    "course":"Computer Science",
    "qualification":"Bachelor / College",
    "date_grad":"06/05/2012",
    "notes":"",
    "deleted":0
  },
  "3":{
    "school_ctr":"schoolctr_3",
    "school":"Siliman University",
    "course":"Information Technology",
    "qualification":"Post Graduate / Master",
    "date_grad":"06/06/2014",
    "notes":"",
    "deleted":0
  }
}

I passed it through AJAX and I was able to save info through this code:

if(!empty($school_details_new)){
  foreach($school_details_new as $key => $value) {
    foreach($value as $school_data => $data_values) {
      if($school_data == "school")
        $s_name = mysql_real_escape_string($data_values);
      if($school_data == "course")
        $s_degree = mysql_real_escape_string($data_values);
      if($school_data == "qualification")
        $s_type = mysql_real_escape_string($data_values);
      if($school_data == "date_grad")
        $s_enddate = mysql_real_escape_string($data_values);
      if($school_data == "notes")
        $s_notes = mysql_real_escape_string($data_values);                      
    }
  $sql = "INSERT INTO `SchoolDetails` (`personID`, `School`, `Type`, `End_Date`, `Degree`, `Notes`) VALUES ($id, '$s_name', '$s_type', '$s_enddate', '$s_degree', '$s_notes');";
  $res_sql = mysql_query ($sql);
  }
}

This is actually working fine with me. I don't have any errors or anything but I do believe there is a more efficient way to do this without having a lot of if statements. Thoughts?

share|improve this question

1 Answer 1

Indeed there is! Use prepared statements instead of manually escaping and inserting into your query!

Please, don't use mysql_* functions in new code. They are no longer maintained and are officially deprecated. See the red box? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial.

//mysqli connection somewhere around here...

if (!empty($school_details_new)) {
    $stmt = mysqli_prepare($conn, "INSERT INTO `SchoolDetails` (`personID`, `School`, `Type`, `End_Date`, `Degree`, `Notes`) VALUES (?, ?, ?, ?, ?, ?)";
    foreach ($school_details as $school) {
        mysqli_stmt_bind_param($stmt, "isssss", $id, $school["school"], $school["qualification"], $school["date_grad"], $school["course"], $school["notes"]);
        mysqli_stmt_execute($stmt);
    }
}
share|improve this answer
1  
+1 for being the first to jump on this, +1 to reiterate the depreciation of mysql_*. –  azngunit81 Jun 30 '14 at 14:28

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.