HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Making a grid of videos

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
makinggridvideos

Problem

In my single-work.php I have some code with slight variations that I feel could be more DRY. When there are only slight variations like this is it ok to repeat part of the code? Or is there another way to write parts of this?

```
// Advanced Custom Fields

// Grid images or videos
$grid_image_1 = get_field('image_1');
$grid_image_2 = get_field('image_2');
$grid_image_3 = get_field('image_3');
$grid_image_4 = get_field('image_4');
$grid_image_5 = get_field('image_5');

// Video embeds
$image_1_video = get_field('image_1_do_you_wish_to_add_a_video');
$video_embed_code_1 = get_field('video_embed_code_1');

$image_2_video = get_field('image_2_do_you_wish_to_add_a_video');
$video_embed_code_2 = get_field('video_embed_code_2');

$image_3_video = get_field('image_3_do_you_wish_to_add_a_video');
$video_embed_code_3 = get_field('video_embed_code_3');

$image_4_video = get_field('image_4_do_you_wish_to_add_a_video');
$video_embed_code_4 = get_field('video_embed_code_4');

$image_5_video = get_field('image_5_do_you_wish_to_add_a_video');
$video_embed_code_5 = get_field('video_embed_code_5');

// General
$about = get_field('about');
$subtitle = get_field('subtitle');

get_template_part( 'template-parts/content', 'carousel' );

?>













" alt="">


" alt="">







" alt="">


" alt="">







" alt="">


Solution

Yes, you are right: you're code is a little wet and could be DRYed some more.

See a pattern in the first section of your code? I do: you have a bunch of variables that you are naming things like $varname_number (mainly the number part). Typically, when you are naming a bunch of variables with numbers in their name that all are going to hold similar things, then you are probably repeating yourself.

Let's look at this section first:

$grid_image_1       = get_field('image_1');
$grid_image_2       = get_field('image_2');
$grid_image_3       = get_field('image_3');
$grid_image_4       = get_field('image_4');
$grid_image_5       = get_field('image_5');


This can be refactored into an array and a loop. The array would store what is currently these five similarly named variables, and a loop would be to set the array values.

Now, I'm not good with PHP, but I think that code would look like this:

$grid_image = array(5);
for($i = 0; $i < count($grid_image); $i++) {
    $grid_image[$i] = get_field('image_' . $i+1);
}


Now, you are not repeating any code.

The same thing can be applied to the next section:

$image_videos = array(5);
$video_embed_codes = array(5);
for($i = 0; $i < count($image_videos); $i++) {
    $image_videos[$i] = get_field('image_' . $i+1 . '_do_you_wish_to_add_a_video');
    $video_embed_codes[$i] = get_field('video_embed_code_' . $i+1);
}



Remember: when you are a series of variables with numbers in them that
are all holding similar values (in this case, they were all holding
the same function's different returns), try using a loop and a few
arrays.

Code Snippets

$grid_image_1       = get_field('image_1');
$grid_image_2       = get_field('image_2');
$grid_image_3       = get_field('image_3');
$grid_image_4       = get_field('image_4');
$grid_image_5       = get_field('image_5');
$grid_image = array(5);
for($i = 0; $i < count($grid_image); $i++) {
    $grid_image[$i] = get_field('image_' . $i+1);
}
$image_videos = array(5);
$video_embed_codes = array(5);
for($i = 0; $i < count($image_videos); $i++) {
    $image_videos[$i] = get_field('image_' . $i+1 . '_do_you_wish_to_add_a_video');
    $video_embed_codes[$i] = get_field('video_embed_code_' . $i+1);
}

Context

StackExchange Code Review Q#113209, answer score: 2

Revisions (0)

No revisions yet.