0

So my controller looks like this:

public function insert(){
//form_validation_rules

if($this->form_validation->run() == FALSE){
//redirect to view
}else{
//get data here $this->input->post()
//insert into table here all others depend on this table to exist
//returns story_id
if($table_id){
// here i have to insert data into 4 more table.

}
}
}

My erd looks like this: Tables

So i have to insert into story table first since the story has to exist before i insert the genre/tags/content warning and finally the chapter since in my form to create a new story you have to add the first chapter too.

An answer to my previous question told me a nested if is bad practice and ive also searched here and saw this: PHP - Nested IF statements

My confusion is, where do i process and insert my data if i were to follow a scheme like that?

Writing this question up to here, my brain cleared up a bit, so i might aswell ask if i am right or in the right direction.

So i'll do it like this:

//process data here
$insert_genre = $this->model->insert_genre($genre);
$insert_tag = $this->model->insert_tags($tags);
$insert_warning = $this->model->insert_warning($content_warning);
$insert_chapter = $this->model->add_chapter($chapter);
if(!insert genre){
//redirect view }
if(!insert tags){
//redirect view }
if(!insert content_warning){
//redirect view }
if(!insert chapter){
//redirect view }
else{ 
//load view
}

EDIT:

This is how my model method for inserting genre/tag/contentwarning looks: theyre all similarly written.

public function new_story_genre($genre){
$inserted = 0;
foreach($genre as $row){
$this->db->insert('story_genre', $row);
$inserted += $this->db->affected_rows();}
if($inserted == count($genre)){
return TRUE;}else{ return FALSE; }
}
John
  • 29
  • 6
  • This looks dangerous, you should do some validations first to prevent false responses for any of the insert queries. If any of them fails, then you run the risk of having duplicate data if user repeats the queries – Chibueze Opata Feb 16 '18 at 10:36
  • Ive had this question previously. My insert bar any special circumstances where the database itself is broken somehow, theres no way the inserts will fail since i processed the data before sending them to the model. Unless somehow someone can just stop the form from submitting midway to the controller and maliciously change/erase the data there, then again i have form validation in my controller and this is just a project not a live website. – John Feb 16 '18 at 10:38
  • So i should run all my inserts before the if statement. So if there was an error inserting tags, the genre/contentwarning/chapter will stil be inserted. I can just then print an error message that there was an error inserting tags? – John Feb 16 '18 at 10:47
  • There is nothing wrong with your approach then. You can remove the variables and return corresponding error messages to the view if any of them fail. – Chibueze Opata Feb 16 '18 at 10:49
  • 2
    Ah, so instead of returning true/false variables i can just return an error message string. – John Feb 16 '18 at 10:50
  • Btw, which answer should i accept. cashbee answered 2minutes earlier but your answer is more detailed. Since both of you helped me, i'm torn. – John Feb 16 '18 at 10:52
  • Deleted my answer. As long as you got the information, you're good – Chibueze Opata Feb 16 '18 at 10:53

1 Answers1

0

Nested if conditions are perfectly acceptable if they are not too many and if it is still easy to read.
You could easily nest your ifs here.

Personally i would use one if in this case:

// if insert errors: redirect view
if(!$insert_genre || !$insert_tags || !$insert_warning || !insert_chapter){
    // redirect view
}

Pro tip: indent everything correctly, it helps wonders with readability of your code.

Edit:
you mentioned error strings in comments. Here is how you could structure your ifs with custom error strings:

if(!$insert_genre = $this->model->insert_genre($genre)){
    redirect_view("Genre insert failed!");
}
if(!$insert_tags = $this->model->insert_tags($tags)){
    redirect_view("Tags insert failed!");
}


public function redirect_view($errorString = ""){
    // redirect view with individual error string
}

if you want you can also return an error string in the insert functions of the model. you have to change this code of the controller a bit but it would definately possible.

kscherrer
  • 5,486
  • 2
  • 19
  • 59