CODEWITHSUNDEEP
phpfunctionglobal
Quiescent
Quiescent

Reputation: 1144

Merging two very similar PHP functions into one

I have two almost identical functions and I am not familiar with PHP but need just a bit of work around. I have several functions as the two shown here and I would like to know how can I merge both of them into one. I use PHP-5.4

function is_checked_gen($block_name, &$curr_rec, $rec_num) {
     global $gen_checked;
     global $gen_list;

     if (in_array($curr_rec[val], $gen_checked)) {
              $curr_rec['checked'] = 'checked';
              $gen_list .= " " . $curr_rec[val];
     } else {
              $curr_rec['checked'] = '';
     }
}

function is_checked_veh($block_name, &$curr_rec, $rec_num) {
     global $veh_checked;
     global $veh_list;

     if (in_array($curr_rec[val], $veh_checked)) {
              $curr_rec['checked'] = 'checked';
              $veh_list .= " " . $curr_rec[val];
     } else {
              $curr_rec['checked'] = '';
     }
}

Upvotes: 0

Views: 77

Answers (4)

Dave Chen
Dave Chen

Reputation: 10975

Well if you're going to have $gen_list, $gen_checked, and then $veh_list, $veh_checked you should make this into a class.

You could even put the is_checked method into the class.

class Type {
    public $checked;
    public $list;
}

function is_checked(Type $type, $block_name, &$curr_rec, $rec_num) {

     if (in_array($curr_rec[val], $type->checked)) {
              $curr_rec['checked'] = 'checked';
              $type->list .= " " . $curr_rec[val];
     } else {
              $curr_rec['checked'] = '';
     }

}

$gen = new Type;
$veh = new Type;

is_checked($veh, ...);
is_checked($gen, ...);

Upvotes: 1

Siva
Siva

Reputation: 2785

In each function, only the corresponding list is getting updated when checked is found in the array. hence pass that list as reference and update it.

function is_checked($block_name, &$curr_rec, $rec_num, $checked, &$list) {

     if (in_array($curr_rec[val], $checked)) {
              $curr_rec['checked'] = 'checked';
              $list .= " " . $curr_rec[val];
     } else {
              $curr_rec['checked'] = '';
     }
}

is_checked($block_name, $curr_rec, $rec_num, $gen_checked, $gen_list);
is_checked($block_name, $curr_rec, $rec_num, $veh_checked, $veh_list);

Upvotes: 1

BruceOverflow
BruceOverflow

Reputation: 576

I guess the question is misplaced. To merge two functions in a first of all you need if you're writing procedural or OOP.

Procedure should be divided into small services and add them to perform complex tasks. OOP can use inheritance but here they serve a solid foundation before proceeding to refactor.

you must specifically identify common issues and refactor the one thing only

example not tested

//refactoring

fucntion is_checked_by($block_name, &$curr_rec, $rec_num, $obj_checked, $list_checked )
{


     if (in_array($curr_rec[val], $obj_checked)) {
              $curr_rec['checked'] = 'checked';
              $list_checked .= " " . $curr_rec[val];
     } else {
              $curr_rec['checked'] = '';
     }
}


function is_checked_gen($block_name, &$curr_rec, $rec_num) {

         is_checked_by($block_name, &$curr_rec, $rec_num, $gen_checked, $gen_list )
}

function is_checked_veh($block_name, &$curr_rec, $rec_num) {
      is_checked_by($block_name, &$curr_rec, $rec_num, $veh_checked, $veh_list )
}

this is just an example and not the best is just to make you understand OOP any funds

Upvotes: 0

Jørgen R
Jørgen R

Reputation: 10806

The thing that stands out are the different global variables that are used in the functions. You shouldn't use globals at all, but that's out of the scope for this answer.

By sending the now global variables into the functions as parameters, you can merge them:

function is_checked($block_name, &$curr_rec, $rec_num, $checked, $list) {
    ....

    return $list;
}

Here, $list (formerly $gen_list and $veh_list) is returned from the function, so you can use this to update your variables from where it's called:

$gen_list = is_checked($block_name, $curr_rec, $rec_num, $gen_checked, $gen_list);
$vac_list = is_checked($block_name, $curr_rec, $rec_num, $vac_checked, $vac_list);

Upvotes: 1

Related Questions