2
\$\begingroup\$

I made a PHP script to check value before update. This script is working fine. But, I wonder how to simplify this. If there are hundreds of columns to be checked then it's hundreds of PHP lines.

This is used for saving a log file for update. So, if data is updated then it stores a log in a table.

public function edit_project_master(){
$no_pro     = 'PRJ-2017-501'; //
$na_pro     = $this->input->post('na_pro'); //value for edit
$ja_pro     = $this->input->post('i_metode'); //value for edit      

$check_pro  = $this->MProject->get_det_pro($no_pro); //get data form table

//checking script
foreach($check_pro as $res_check_pro){

    if($na_pro != $res_check_pro['NAMA_PROJECT']){  //IF VALUE DIFFERENT
        $col_name;

        foreach($res_check_pro as $key_pro => $val_pro){
            if($res_check_pro['NAMA_PROJECT'] == $val_pro){
                $col_name = $key_pro;            //GET COLUMN NAME
            }
        }

        $data['ACTIVITY']       = "UPDATE";
        $data['OLD_VALUE']      = $res_check_pro['NAMA_PROJECT'];
        $data['NEW_VALUE']      = $na_pro;
        $data['COL_AFFECTED']   = $col_name;
        $data['TABLE_AFFECTED'] = "PM_REQUIREMENT_PROJECT";  //How to get table name automatically??
        $data['ANNOTATION']     = NULL;
        $data['EXECUTOR']       = $this->session->userdata('nip');
        $data['CHANGED_ON']     = date("Y-m-d H:i:s");
        $data['CHANGED_BY']     = $_SERVER['REMOTE_ADDR'];
        $data['STATUS']         = 1;

        $this->MProject->ins_system_log($data);             
    }

    if($ja_pro != $res_check_pro['JADWAL_PROJECT']){
        $col_name;

        foreach($res_check_pro as $key_pro => $val_pro){
            if($res_check_pro['JADWAL_PROJECT'] == $val_pro){
                $col_name = $key_pro;
            }
        }

        $data['ACTIVITY']       = "UPDATE";
        $data['OLD_VALUE']      = $res_check_pro['JADWAL_PROJECT'];
        $data['NEW_VALUE']      = $na_pro;
        $data['COL_AFFECTED']   = $col_name;
        $data['TABLE_AFFECTED'] = "PM_REQUIREMENT_PROJECT"; //How to get table name automatically??
        $data['ANNOTATION']     = NULL;
        $data['EXECUTOR']       = $this->session->userdata('nip');
        $data['CHANGED_ON']     = date("Y-m-d H:i:s");
        $data['CHANGED_BY']     = $_SERVER['REMOTE_ADDR'];
        $data['STATUS']         = 1;

        $this->MProject->ins_system_log($data);             
    }


}


}
\$\endgroup\$
2
\$\begingroup\$

You can avoid repeating whole blocks of code by isolating the dynamic components of your function and assign them to an array. Then as you wish to modify the fields/columns to be checked, you merely need to update the array and not the functional/checking parts.

public function edit_project_master(){
    $no_pro     = 'PRJ-2017-501';
    $check_pro  = $this->MProject->get_det_pro($no_pro); //get data form table

    $checklist=[
        ['input'=>'na_pro','column'=>'NAMA_PROJECT','table'=>'PM_REQUIREMENT_PROJECT'],
        ['input'=>'i_metode','column'=>'JADWAL_PROJECT','table'=>'PM_REQUIREMENT_PROJECT']
    ];

    //checking script
    foreach($check_pro as $res_check_pro){
        foreach($checklist as $listrow){
            $pro = $this->input->post($listrow['input']); //value for edit
            if($pro != $res_check_pro[$listrow['column']]){  //IF VALUE DIFFERENT
                foreach($res_check_pro as $key_pro => $val_pro){
                    if($res_check_pro[$listrow['column']] == $val_pro){
                        $col_name = $key_pro;            //GET COLUMN NAME
                        break;  // I assume you want to break as soon as you find a match
                    }
                }
                $data['ACTIVITY']       = "UPDATE";
                $data['OLD_VALUE']      = $res_check_pro[$listrow['column']];
                $data['NEW_VALUE']      = $pro;
                $data['COL_AFFECTED']   = $col_name;
                $data['TABLE_AFFECTED'] = $listrow['table'];  // I'm not sure how you mean "automatically", this is "dynamically"
                $data['ANNOTATION']     = NULL;
                $data['EXECUTOR']       = $this->session->userdata('nip');
                $data['CHANGED_ON']     = date("Y-m-d H:i:s");
                $data['CHANGED_BY']     = $_SERVER['REMOTE_ADDR'];
                $data['STATUS']         = 1;
                $this->MProject->ins_system_log($data);
            }
        }
    }
}

Also...

I added break; to the innermost foreach() assuming there will only be one match. This is best practice because it avoids doing pointless iterations.

I made the value for $data['TABLE_AFFECTED'] "dynamic" (modifiable via the $checklist array). I don't know if this is what you had in mind by "automatic".

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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