Is it better to validate and put an array value into a variable at the top of a file or to check the array value directly right before output in the file?
I work at a WordPress shop and we have a function that some developers commonly use to check the value of indexes in an array of custom field values:
//Check if an array has a key and return its value if so
function pkav($arr,$key){
return isset($arr[$key]) ? $arr[$key] : false;
}
This leads to the following code in a template:
<?php
$options = get_fields('options');
$footer_text = pkav($options, 'footer_text');
$footer_button = pkav($options, 'footer_button');
?>
<!-- Other code here -->
<?php if ( $footer_text || $footer_button ) : ?>
<div class="col-12 col-lg-6">
<?php if( $footer_text ) : ?>
<h4><?php echo esc_html( $footer_text ); ?></h4>
<?php endif; ?>
<?php if( $footer_button ) : ?>
<?php pk_output_button( $footer_button ); ?>
<?php endif; ?>
</div>
<?php endif; ?>
Some devs think this is more readable and a better way to code and check values/variables. It also allows us to neatly declare all variables at the top of a file so we immediately know what variables are used in a given template/template part. We have other devs who think pkav is unnecessary and an incorrect way to use variables. They'd rather see code like this:
<?php
$options = get_fields('options');
?>
<!-- Other code here -->
<?php if ( ! empty( $options['footer_text'] ) || ! empty( $options['footer_button'] ) ) : ?>
<div class="col-12 col-lg-6">
<?php if( ! empty( $options['footer_text'] ) ) : ?>
<h4><?php echo esc_html( $options['footer_text'] ); ?></h4>
<?php endif; ?>
<?php if( ! empty( $options['footer_button'] ) ) : ?>
<?php pk_output_button( $options['footer_button'] ); ?>
<?php endif; ?>
</div>
<?php endif; ?>
Is there a right answer on what should be used in terms of php/wordpress/programming best practices/standards? The pkav functions seems to create more readable code by eliminating duplicate empty()
checks and shortening each line but using only empty()
checks seems to prevent creating extra variables.
1 Answer 1
I would not use the pkav
function. Beside the fact that the name means nothing (which was stated in the comments) it also restricts the default value to false
.
This is wrong when you might expect strings or something else for a certain variable.
From the looks of it, the footer text and button vars look like strings.
I would go with this
<?php
$options = get_fields('options');
$footer_text = $options['footer_text'] ?? '';
$footer_button = $options['footer_button'] ?? '';
?>
I would also go with declaring all variables at the top of the file. It makes the code more readable and will allow a frontend developer to focus on the html part instead of trying to navigate in the if
statements.
<?php if ($something) :?>
<div><?= esc_html($something) ;?></div>
<?php endif;?>
is easier to read than
<?php if (!empty( $options['footer_text'])) : ?>
<h4><?php echo esc_html($options['footer_text']); ?></h4>
<?php endif; ?>
pkav
is a terrible function name. Or abbreviated: ew,pkav
is a trbl fnn. See how ridiculously difficult it Is for a reader to understand abbreviated text? Its the same with source codes... \$\endgroup\$