2
\$\begingroup\$

I'm currently working on an approval system right now using PHP (Codeigniter) and I have this method of getting all the requests according to your department group and position depth.

The checking of the requests is based upon the user's position depth. 6 being the lowest and 1 being the highest. So basically there will be a maximum of 5 people to check the request. For example my depth is 6, line1 will be 5, line2 will be 4 and so on. The system will find someone with position depths like that and same department group. Lines 3 and 2 will decide to send it to ceo (1) or not so the default is only until 2.

Some of my database columns Some database columns

It is designed that people above the line can check the request even if the lower lines havent checked it yet like for example maybe line1 havent checked it yet because he/she is on vacation and line3 cannot wait already so he/she decides to process the request because it is important. Now if the above line processes it already, lower lines cannot process it anymore so they will only be allowed to view it.

Now here comes my code of nested for loops in checking out all these things that I said. People say nested for loops, especially as many as mine is not a good practice. Is my for loop gonna have problems when it comes to large data? If so how do I alter my code without altering my database for this?

This is my Model

public function getRequestsToProcess($i,$depth,$departmentGroup)
{
    $this->db->select('*, ns_request_info.idx as request_idx, ns_staff_info.idx as staff_idx');    
    $this->db->from('ns_request_info');
    $this->db->join('ns_staff_info', 'ns_request_info.requested_by_idx = ns_staff_info.member_idx');
    $this->db->where('line'.$i.'', $depth);
    $this->db->where('line'.$i.'_action', 'PENDING');
    if($departmentGroup!='HIGHER TIER'){ $this->db->where('ns_staff_info.department_group', $departmentGroup); }
    $this->db->where('status', 'PENDING');
    $this->db->or_where('status', 'ONGOING');
    $this->db->where('line'.$i.'', $depth);
    $this->db->where('line'.$i.'_action', 'PENDING');
    if($departmentGroup!='HIGHER TIER'){ $this->db->where('ns_staff_info.department_group', $departmentGroup); }
    $query = $this->db->get();
    return $query->result_array();
}

This is my method with nested for loops

public function process()
{
    $data = new stdClass;
    $data->param_menu = 'request';

    $staffInfo = $this->staffinfo_model->selectItem(array('member_idx'=>$this->session->userdata('member_index')));

    for ( $i=1 ; $i<=5 ; $i++) {
        /*get lines with user depth and if same department group*/
        ${"line" . $i} = $this->staffrequisition_model->getRequestsToProcess($i,$staffInfo->position_depth,$staffInfo->department_group);

        for ($x = 0 ; $x < count(${"line" . $i}) ; $x++) {
            $counter = 0;
            for ($y = $i + 1 ; $y <=5 ; $y++) {
                if (${"line" . $i}[$x]['line'.$y.'_action']!=='PENDING' && ${"line" . $i}[$x]['line'.$y.'_action']!=='') {
                    $counter++;
                    break; 
                } 
            }
            if ($counter > 0) {
                ${"line" . $i}[$x]['is_checked'] = 1;
            } else {
                ${"line" . $i}[$x]['is_checked'] = 0;
            }
        }
    }

    $data->param_requests = array_merge($line1, $line2, $line3, $line4, $line5);

    $this->load->view('dashboard/staff/process_view', $data);
}

Now is_checked being the marker if the request is already checked in upper lines. I hope you guys can give me any tips on how to optimize my code.

\$\endgroup\$
  • 2
    \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$ – Ludisposed Jul 31 '18 at 7:46
1
\$\begingroup\$

I am able to remove one for loop in my code with the following:

I changed my method in the Controller

public function process()
{
    $data = new stdClass;
    $data->param_menu = 'request';

    $staffInfo = $this->staffinfo_model->selectItem(array('member_idx'=>$this->session->userdata('member_index')));

    $requests = $this->staffrequisition_model->getRequestsToProcess($staffInfo->position_depth,$staffInfo->department_group);

    foreach ($requests as &$item) {
        for ($y = 1; $y <=5 ; $y++) {
            if ($item->{'line'.$y.'_action'}!=='PENDING' && $item->{'line'.$y.'_action'}!==''){
                $item->is_checked = TRUE;
                break;
            } else {
                $item->is_checked = FALSE;
            }
        }
    }

    $data->param_requests = $requests;

    $this->load->view('dashboard/staff/process_view', $data);
}

I changed my method in the Model

public function getRequestsToProcess($depth,$departmentGroup)
{
    $this->db->select('*, ns_request_info.idx as request_idx, ns_staff_info.idx as staff_idx');    
    $this->db->from('ns_request_info');
    $this->db->join('ns_staff_info', 'ns_request_info.requested_by_idx = ns_staff_info.member_idx');
    $this->db->where("((ns_request_info.line1=".$depth." AND ns_request_info.line1_action='PENDING') OR (ns_request_info.line2=".$depth." AND ns_request_info.line2_action='PENDING') OR (ns_request_info.line3=".$depth." AND ns_request_info.line3_action='PENDING') OR (ns_request_info.line4=".$depth." AND ns_request_info.line4_action='PENDING') OR (ns_request_info.line5=".$depth." AND ns_request_info.line5_action='PENDING'))");
    $this->db->where("(ns_request_info.status='PENDING' OR ns_request_info.status='ONGOING')");
    if($departmentGroup!='HIGHER TIER'){ $this->db->where('ns_staff_info.department_group', $departmentGroup); }
    $this->db->order_by('ns_request_info.idx', 'desc');
    $query = $this->db->get();
    return $query->result();
}

Now I dont have to loop my query five times. Instead I get it with just one query and loop through the result.

\$\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.