2
\$\begingroup\$

Im using a bunch of ACF fields for my WooCommerce email templates and for this example Im calling these in my email_header template. Im using this with WPML, and need to call each language settings. The code works, but seems like it should be a way to write this better.

This is used to set and fetch the correct language based on meta data on orders. Works well. For this I created an array to shorten it down for each if/else statement. NL is the default language.

$language = get_post_meta( get_the_ID(), 'wpml_language', true );
if ($language == 'en') {

For the front, this is used to fetch the right language:

} elseif (ICL_LANGUAGE_CODE=='en') {

For this statement to get the correct language, I need to ad a language prefix to each ACF variable, so it doesn't work to use the variable from the array, it defaults to NL then:

$sitelogo = get_field('en_logo', 'options'); 

How can I shorten this down? full code:

 $header_variabls_one = $sitelogo = get_field('logo', 'options'); 
     $header_variabls_two = $emailLogoLink = get_field('email_logo_link', 'options');
     $header_variabls_three = $sitelogo = get_field('logo', 'options'); 
     $header_variabls_four = $header_link_one = get_field('header_links_link_one', 'options');
     $header_variabls_five = $header_link_one_url = get_field('header_links_link_one_url', 'options');
     $header_variabls_six =  $header_link_two = get_field('header_links_link_two', 'options');
     $header_variabls_seven = $header_link_two_url = get_field('header_links_link_two_url', 'options');
     $header_variabls_eight = $header_link_three = get_field('header_links_link_three', 'options');
     $header_variabls_nine = $header_link_three_url = get_field('header_links_link_three_url', 'options');
     $header_variabls_ten = $header_link_four = get_field('header_links_link_four', 'options');
     $header_variabls_eleven = $header_link_four_url = get_field('header_links_link_four_url', 'options');
     $header_variabls_twelve = $ups1 = get_field('header_usps_usp_1', 'options');
     $header_variabls_thirteen = $ups2 = get_field('header_usps_usp_2', 'options');
     $header_variabls_fourtneen = $ups3 = get_field('header_usps_usp_3', 'options');    
    
    
$header_variabls = array (
     $header_variabls_one,
     $header_variabls_two,
     $header_variabls_three,
     $header_variabls_four,
     $header_variabls_five,
     $header_variabls_six,
     $header_variabls_seven,
     $header_variabls_eight,
     $header_variabls_nine,        
     $header_variabls_ten,
     $header_variabls_eleven, 
     $header_variabls_twelwe,
     $header_variabls_thirteen,
     $header_variabls_fourteen,
);
    
       
    global $post;
    global $sitepress;
    $postid = $post->ID;  
    $language = get_post_meta( get_the_ID(), 'wpml_language', true );
    if ($language == 'en') {
        add_filter('acf/settings/current_language', function(){return 'en';});
        $header_variabls;
    } elseif ($language == 'nl') {
        add_filter('acf/settings/current_language', function(){return 'nl';});
        $header_variabls;
    } elseif ($language == 'de') {
        add_filter('acf/settings/current_language', function(){return 'de';});
        $header_variabls;
    } elseif (ICL_LANGUAGE_CODE=='en') {
        $sitepress->switch_lang( 'en', true );
        do_action( 'wpml_switch_language', "en" );
        $sitelogo = get_field('en_logo', 'options'); 
        $emailLogoLink = get_field('en_email_logo_link', 'options');
        $sitelogo = get_field('en_logo', 'options'); 
        $header_link_one = get_field('en_header_links_link_one', 'options');
        $header_link_one_url = get_field('en_header_links_link_one_url', 'options');
        $header_link_two = get_field('en_header_links_link_two', 'options');
        $header_link_two_url = get_field('en_header_links_link_two_url', 'options');
        $header_link_three = get_field('en_header_links_link_three', 'options');
        $header_link_three_url = get_field('en_header_links_link_three_url', 'options');
        $header_link_four = get_field('en_header_links_link_four', 'options');
        $header_link_four_url = get_field('en_header_links_link_four_url', 'options');
        $ups1 = get_field('en_header_usps_usp_1', 'options');
        $ups2 = get_field('en_header_usps_usp_2', 'options');
        $ups3 = get_field('en_header_usps_usp_3', 'options');   
    } elseif (ICL_LANGUAGE_CODE=='de') {
        $sitelogo = get_field('de_logo', 'options'); 
        $emailLogoLink = get_field('de_email_logo_link', 'options');
        $sitelogo = get_field('de_logo', 'options'); 
        $header_link_one = get_field('de_header_links_link_one', 'options');
        $header_link_one_url = get_field('de_header_links_link_one_url', 'options');
        $header_link_two = get_field('de_header_links_link_two', 'options');
        $header_link_two_url = get_field('de_header_links_link_two_url', 'options');
        $header_link_three = get_field('de_header_links_link_three', 'options');
        $header_link_three_url = get_field('de_header_links_link_three_url', 'options');
        $header_link_four = get_field('de_header_links_link_four', 'options');
        $header_link_four_url = get_field('de_header_links_link_four_url', 'options');
        $ups1 = get_field('de_header_usps_usp_1', 'options');
        $ups2 = get_field('de_header_usps_usp_2', 'options');
        $ups3 = get_field('de_header_usps_usp_3', 'options'); 
    } elseif (ICL_LANGUAGE_CODE=='nl') {
        $sitepress->switch_lang( 'nl', true );
        do_action( 'wpml_switch_language', "nl" );      
        $header_variabls;
    }
\$\endgroup\$
3
  • 1
    \$\begingroup\$ A CodeReview title must not include the kind of review that you are seeking. The title must only describe what your script does. You might like to add a wordpress tag to you question. Please take the tour and edit your question. \$\endgroup\$ Oct 27 '21 at 20:06
  • \$\begingroup\$ Welcome to Code Review! Is $header_variabls_fourtneen (on the 14th line) supposed to be $header_variabls_fourteen (like it is on the 31st line)? \$\endgroup\$ Oct 27 '21 at 20:16
  • \$\begingroup\$ Yes! Miss-spelled. Thanks! \$\endgroup\$ Oct 28 '21 at 7:28
3
\$\begingroup\$

Although I am pretty new to PHP/WordPress myself, one item I saw right away to make your code prettier is spacing.

From the WordPress PHP Coding Standards section on indentation

Your indentation should always reflect logical structure. Use real tabs and not spaces, as this allows the most flexibility across clients.

Exception: if you have a block of code that would be more readable if things are aligned, use spaces:

[tab]$foo   = 'somevalue';
[tab]$foo2  = 'somevalue2';
[tab]$foo34 = 'somevalue3';
[tab]$foo5  = 'somevalue4';

Also for prettiness you should have a lot more spaces: Always put spaces after commas, and on both sides of logical, comparison, string and assignment operators. (from that same page from WordPress)

Another quick win is 'options' is repeated throughout the page. You can set a variable at the top to repeat this.

I also removed the double variable as it didn't seem necessary but I may not know something there so have caution. It just didn't seem necessary.

Lastly, you spelled out numbers instead of writing them, this was a lot of extra characters you could have just used numbers in.

<?php

$o = 'options';

$sitelogo               = get_field( 'logo', $o ); 
$emailLogoLink          = get_field( 'email_logo_link', $o );
$sitelogo               = get_field( 'logo', $o ); 
$header_link_1          = get_field( 'header_links_link_1', $o );
$header_link_1_url      = get_field( 'header_links_link_1_url', $o );
$header_link_2          = get_field( 'header_links_link_2', $o );
$header_link_2_url      = get_field( 'header_links_link_2_url', $o );
$header_link_3          = get_field( 'header_links_link_3', $o );
$header_link_3_url      = get_field( 'header_links_link_3_url', $o );
$header_link_4          = get_field( 'header_links_link_4', $o );
$header_link_4_url      = get_field( 'header_links_link_4_url', $o );
$ups1                   = get_field( 'header_usps_usp_1', $o );
$ups2                   = get_field( 'header_usps_usp_2', $o );
$ups3                   = get_field( 'header_usps_usp_3', $o );    
    
    
$header_variabls = array (
    $sitelogo,             
    $emailLogoLink,        
    $sitelogo,             
    $header_link_1,      
    $header_link_1_url,  
    $header_link_2,      
    $header_link_2_url,  
    $header_link_3,    
    $header_link_3_url,
    $header_link_4,     
    $header_link_4_url, 
    $ups1,                 
    $ups2,                 
    $ups3,                 
);
    
       
    global $post;
    global $sitepress;
    $postid     = $post->ID;  
    $language   = get_post_meta( get_the_ID(), 'wpml_language', true );
    if ( $language == 'en' ) {
        add_filter( 'acf/settings/current_language', function(){ return 'en'; } );
        $header_variabls;
    } elseif ( $language == 'nl' ) {
        add_filter( 'acf/settings/current_language', function(){ return 'nl'; } );
        $header_variabls;
    } elseif ( $language == 'de' ) {
        add_filter( 'acf/settings/current_language', function(){ return 'de'; } );
        $header_variabls;
    } elseif ( ICL_LANGUAGE_CODE=='en' ) {
        $sitepress->switch_lang( 'en', true );
        do_action( 'wpml_switch_language', "en" );
        $sitelogo               = get_field('en_logo', $o); 
        $emailLogoLink          = get_field('en_email_logo_link', $o);
        $sitelogo               = get_field('en_logo', $o); 
        $header_link_1          = get_field('en_header_links_link_1', $o);
        $header_link_1_url      = get_field('en_header_links_link_1_url', $o);
        $header_link_2          = get_field('en_header_links_link_2', $o);
        $header_link_2_url      = get_field('en_header_links_link_2_url', $o);
        $header_link_3          = get_field('en_header_links_link_3', $o);
        $header_link_3_url      = get_field('en_header_links_link_3_url', $o);
        $header_link_4          = get_field('en_header_links_link_4', $o);
        $header_link_4_url      = get_field('en_header_links_link_4_url', $o);
        $ups1                   = get_field('en_header_usps_usp_1', $o);
        $ups2                   = get_field('en_header_usps_usp_2', $o);
        $ups3                   = get_field('en_header_usps_usp_3', $o);   
    } elseif ( ICL_LANGUAGE_CODE=='de' ) {
        $sitelogo               = get_field('de_logo', $o); 
        $emailLogoLink          = get_field('de_email_logo_link', $o);
        $sitelogo               = get_field('de_logo', $o);
        $header_link_1          = get_field('de_header_links_link_1', $o);
        $header_link_1_url      = get_field('de_header_links_link_1_url', $o);
        $header_link_2          = get_field('de_header_links_link_2', $o);
        $header_link_2_url      = get_field('de_header_links_link_2_url', $o);
        $header_link_3          = get_field('de_header_links_link_3', $o);
        $header_link_3_url      = get_field('de_header_links_link_3_url', $o);
        $header_link_4          = get_field('de_header_links_link_4', $o);
        $header_link_4_url      = get_field('de_header_links_link_4_url', $o);
        $ups1                   = get_field('de_header_usps_usp_1', $o);
        $ups2                   = get_field('de_header_usps_usp_2', $o);
        $ups3                   = get_field('de_header_usps_usp_3', $o); 
    } elseif ( ICL_LANGUAGE_CODE=='nl' ) {
        $sitepress->switch_lang( 'nl', true );
        do_action( 'wpml_switch_language', "nl" );      
        $header_variabls;
    }

    ?>
\$\endgroup\$
1
  • 2
    \$\begingroup\$ I'd highly suggest using PSR instead of WP's code standards. At the end of the day you are writing code in PHP regardless of what framework or alike you are using. You never know, a piece of code you used in a WP project, you might use it somewhere else not to mention actually following PHP / Core language standards should always be preferred. \$\endgroup\$ Oct 29 '21 at 19:49
3
\$\begingroup\$

Some points to consider:

  • You have a duplicate header value, site logo appears twice
  • Inconsistent variable names: you are using sitelogo, emailLogoLink, and header_link_one
  • It might be worth considering checking lowercase values of strings in case you get 'NL' instead of 'nl'

You can also use the header variables to store the name of the field, and then grab them later. This lets you prepend the language code to them to make it more DRY.

I used a switch statement for the check for $ICL_LANGUAGE_CODE as I can take advantage of fall-through to not update header fields for NL.

Note: the code below is untested, and may very well not work.

$headerVariables = [
    'logo',
    'email_logo_link',
    'logo',
    'header_links_link_one',
    'header_links_link_one_url',
    'header_links_link_two',
    'header_links_link_two_url',
    'header_links_link_three',
    'header_links_link_three_url',
    'header_links_link_four',
    'header_links_link_four_url',
    'header_usps_usp_1',
    'header_usps_usp_2',
    'header_usps_usp_3'];

global $post;
global $sitepress;
$postid = $post->ID;
$language = get_post_meta( get_the_ID(), 'wpml_language', true );
if (in_array($language, ['en', 'nl', 'de'])) {
    add_filter('acf/settings/current_language', function(){return $language;});
} else {
    switch ($ICL_LANGUAGE_CODE) {
        case 'en':
        case 'de':
            foreach ($headerVariables as $key => $value) {
                $headerVariables[$key] = $ICL_LANGUAGE_CODE . '_' . $value;
            }
        case 'nl':
            $sitepress->switch_lang( $ICL_LANGUAGE_CODE, true );
            do_action( 'wpml_switch_language', $ICL_LANGUAGE_CODE);
    }
}

// Get all header variables
foreach ($headerVariables as $key => $value) {
    $headerVariables[$key] = get_field($value, 'options');
}

// set variables in case we need them later
$sitelogo = $headerVariables[0];
$emailLogoLink = $headerVariables[1];
$sitelogo = $headerVariables[2];
$header_link_one = $headerVariables[3];
$header_link_one_url = $headerVariables[4];
$header_link_two = $headerVariables[5];
$header_link_two_url = $headerVariables[6];
$header_link_three = $headerVariables[7];
$header_link_three_url = $headerVariables[8];
$header_link_four = $headerVariables[9];
$header_link_four_url = $headerVariables[10];
$ups1 = $headerVariables[11];
$ups2 = $headerVariables[12];
$ups3 = $headerVariables[13];
\$\endgroup\$
7
  • \$\begingroup\$ Are you sure about that switch() block? I don't see any breaks. I think I'd use a default case instead of having to maintain an in_array() function and a switch() block. \$\endgroup\$ Oct 31 '21 at 4:23
  • \$\begingroup\$ @mickmackusa taken from the PHP manual: If you don't write a break statement at the end of a case's statement list, PHP will go on executing the statements of the following case. \$\endgroup\$
    – Jason
    Nov 1 '21 at 1:33
  • \$\begingroup\$ So you mean that en and de should also fall through to nl? If so, then I don't see any need to write the two lines of code under nl -- just write them after the switch block. \$\endgroup\$ Nov 1 '21 at 1:38
  • \$\begingroup\$ As far as what I'd use there. I'm not 100% sure I'd use what I put either. But either way multiple if or switch statements are needed as you need to check multiple variables. I think it's a matter of preference, and if working with a team would try to align my code with existing code. \$\endgroup\$
    – Jason
    Nov 1 '21 at 1:49
  • \$\begingroup\$ @mickmackusa Yeah, they should also fall through to nl but nl language should not run the code for en and de. So we have the nl in there to show where the nl code should start from. \$\endgroup\$
    – Jason
    Nov 1 '21 at 1:51

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.