CODEWITHSUNDEEP
phprefactoringconditional-statements
mmarquez
mmarquez

Reputation: 147

Refactoring Conditional PHP

I'm wondering if there is another way to refactor my conditionals if/else statements. I feel I'm repeating the same thing over and over,

Here is a snippet (Keep in mind is wayyyy longer than that, but it follows the same principle) I could do a switch statement but it does not reduce the total code quantity.

I just want to get a second opinion of the best approach for this code to go into production. Also important to mention that the statements that I'm comparing $screen->id; are most likely to be dynamically generated if the user selects the checkbox, but that is outside of the scope of the question. 

    //check admin screen
    $screen = get_current_screen();
    if ( $screen->id === 'topic') {

        $in['content_css'] = get_template_directory_uri() . "/build/styles/tiny-mce-editor.css";
        $in['block_formats'] = $topics_blocks;
        return $in;
    }
    elseif ( $screen->id === 'provider-jobs')  {

        $in['content_css'] = get_template_directory_uri() . "/build/styles/tiny-mce-editor.css";
        $in['block_formats'] = $providers_blocks;
        return $in;
    }
    //for all the page options
    else {

        $in['block_formats'] = $global_blocks;
        return $in;
    }

}

Thanks! Any guidance is appreciated.

UPDATE! Here is the refactored code, a little bit more cleaner! And complete so the whole context shows. 

//check admin screen
$screen = get_current_screen();

//global ones
$in['block_formats'] = $global_blocks;
$in['toolbar1'] = $global_toolbar;

if ( $screen->id === 'topic') {
    $in['block_formats'] = $topics_blocks;
    $in['toolbar1'] = $topics_toolbar;

} elseif ( $screen->id === 'forum')  {
    $in['block_formats'] = $forums_blocks;
    $in['toolbar1'] = $forums_toolbar;

} elseif ( $screen->id === 'post')  {
    $in['block_formats'] = $blogs_blocks;
    $in['toolbar1'] = $blogs_toolbar;

} elseif ( $screen->id === 'jobs')  {
    $in['block_formats'] = $jobs_blocks;
    $in['toolbar1'] = $jobs_toolbar;

}
elseif ( $screen->id === 'provider-jobs')  {
    $in['block_formats'] = $providers_blocks;
    $in['toolbar1'] = $providers_toolbar;

}

return $in;

Upvotes: 0

Views: 979

Answers (1)

cFreed
cFreed

Reputation: 4474

You might register key/value pairs all known options/blocks. So your code becomes straight reduced, like this:

  $options = [
    'topic'         => $topics_blocks,
    'provider_jobs' => $providers_blocks,
    // ...
  ];
  //check admin screen
  $screen = get_current_screen();
  if (array_key_exists($screen->id, $options) {
    $in['content_css'] = get_template_directory_uri() . "/build/styles/tiny-mce-editor.css";
    $in['block_formats'] = $options[$screen->id];
  } else {
    $in['block_formats'] = $global_blocks;
  }
  return $in;

Edit, applies to the 2nd version of the OP

Here the problem appears a bit different, so here is another solution:

$options = [
  'topic' => 'topics',
  'forum' => 'forums',
  'post'  => 'blogs',
  'jobs'  => 'jobs',
  'provider_jobs' => 'providers',
];
//check admin screen
$screen = get_current_screen();

//global ones
$in['block_formats'] = $global_blocks;
$in['toolbar1'] = $global_toolbar;
// variable ones
if (array_key_exists($screen->id, $options) {
  $in['block_formats'] = ${$options[$screen->id] . '_blocks'};
  $in['toolbar1'] = $options[$screen->id] . '_toolbar'};
}
return $in;

Upvotes: 2

Related Questions