3
\$\begingroup\$

As a beginner in php, I welcome you feedback on improving or simplifying the following php code that generates this responsive image html (image sizes and formats are auto-generated using Gulp). For example, is having three nested foreach loops bad practice?

Generated HTML:

<figure> <picture> <source media="(orientation: portrait)" type="image/avif" srcset="assets/img/faith/faith-3x4-xs.avif 576w, assets/img/faith/faith-3x4-sm.avif 768w, assets/img/faith/faith-3x4-md.avif 992w, assets/img/faith/faith-3x4-lg.avif 1200w, assets/img/faith/faith-3x4-xl.avif 1400w, assets/img/faith/faith-3x4-xxl.avif 2048w"> <source media="(orientation: portrait)" type="image/webp" srcset="assets/img/faith/faith-3x4-xs.webp 576w, assets/img/faith/faith-3x4-sm.webp 768w, assets/img/faith/faith-3x4-md.webp 992w, assets/img/faith/faith-3x4-lg.webp 1200w, assets/img/faith/faith-3x4-xl.webp 1400w, assets/img/faith/faith-3x4-xxl.webp 2048w"> <source media="(orientation: portrait)" type="image/jpg" srcset="assets/img/faith/faith-3x4-xs.jpg 576w, assets/img/faith/faith-3x4-sm.jpg 768w, assets/img/faith/faith-3x4-md.jpg 992w, assets/img/faith/faith-3x4-lg.jpg 1200w, assets/img/faith/faith-3x4-xl.jpg 1400w, assets/img/faith/faith-3x4-xxl.jpg 2048w"> <source media="(orientation: landscape)" type="image/avif" srcset="assets/img/faith/faith-3x2-xs.avif 576w, assets/img/faith/faith-3x2-sm.avif 768w, assets/img/faith/faith-3x2-md.avif 992w, assets/img/faith/faith-3x2-lg.avif 1200w, assets/img/faith/faith-3x2-xl.avif 1400w, assets/img/faith/faith-3x2-xxl.avif 2048w"> <source media="(orientation: landscape)" type="image/webp" srcset="assets/img/faith/faith-3x2-xs.webp 576w, assets/img/faith/faith-3x2-sm.webp 768w, assets/img/faith/faith-3x2-md.webp 992w, assets/img/faith/faith-3x2-lg.webp 1200w, assets/img/faith/faith-3x2-xl.webp 1400w, assets/img/faith/faith-3x2-xxl.webp 2048w"> <source media="(orientation: landscape)" type="image/jpg" srcset="assets/img/faith/faith-3x2-xs.jpg 576w, assets/img/faith/faith-3x2-sm.jpg 768w, assets/img/faith/faith-3x2-md.jpg 992w, assets/img/faith/faith-3x2-lg.jpg 1200w, assets/img/faith/faith-3x2-xl.jpg 1400w, assets/img/faith/faith-3x2-xxl.jpg 2048w"> <img src="assets/img/faith/faith-3x2-lg.jpg" width="1200" height="800" alt="Earth Day, Victoria, BC"> </picture> </figure> 

Php code:

 <? $image= [ 'name' => 'faith', 'srcset' => [ 'sizes' => [ 'size' => ['xs','sm','md','lg','xl','xxl'], 'vw' => [ 576, 768, 992, 1200, 1400, 2048 ] ], 'ratio' => ['portrait' => '3x4','landscape' => '3x2' ], 'format' => ['avif', 'webp','jpg'] ], 'caption' => 'Earth Day, Victoria, BC', ]; $sizes = array_combine($image['srcset']['sizes']['size'], $image['srcset']['sizes']['vw']); ?> <figure> <picture> <? foreach ($image['srcset']['ratio'] as $orientation => $ratio): $basePath = "assets/img/{$image['name']}/{$image['name']}-{$ratio}"; foreach($image['srcset']['format'] as $format): $comma = ''; $source = "<source media=\"(orientation: ${orientation})\" type = \"image/{$format}\" srcset=\""; foreach($sizes as $size => $vw): $source .= "{$comma}{$basePath}-{$size}.{$format} {$vw}w"; $comma=', '; endforeach; echo $source .= '">'; endforeach; endforeach; ?> <img src="<?=$basePath?>-lg.jpg" width="1200" height="800" alt="<?=$image['caption']?>"> </picture> </figure> 
\$\endgroup\$

    1 Answer 1

    2
    \$\begingroup\$

    Personally, I don't like that much logic in the template. Even hate it. The code becomes unreadable. In such a case I would rather move all this intricate logic into the PHP part, doing some preprocessing that will result in the plain array, so the end code would be like this

    <figure> <picture> <?php foreach ($image['srcset'] as $row): ?> <source media="(orientation: <?=$row['orientation']?>)" type="image/<?=$row['format']?>" srcset="<?=$row['srcset'] ?>"> <?php endforeach ?> <img src="<?=$imagePath?>lg.jpg" width="1200" height="800" alt="<?=$image['caption']?>"> </picture> </figure> 

    The preprocessing PHP code should be exactly like your current code, just collecting the data into array instead of echoing it

    $new = []; foreach ($image['srcset']['ratio'] as $orientation => $ratio) { $basePath = "assets/img/{$image['name']}/{$image['name']}-{$ratio}"; foreach($image['srcset']['format'] as $format) { $srcset = ''; foreach($sizes as $size => $vw) { $srcset .= $srcset ? ",":""; $srcset .= "{$basePath}-{$size}.{$format} {$vw}w"; } $new[] = [ 'orientation' => $orientation, 'format' => $format, 'srcset' => $srcset, ]; } } 

    another possibility is to write sort of a helper function

    <figure> <picture> <?php foreach ($image['srcset']['ratio'] as $orientation => $ratio): ?> <?php foreach($image['srcset']['format'] as $format): ?> <source media="(orientation: <?=$orientation?>)" type="image/<?=$format?>" srcset="<?=getsrcset($image, $ratio, $format, $key)"> <?php endforeach ?> <? endforeach ?> <img src="<?=$imagePath?>lg.jpg" width="1200" height="800" alt="<?=$image['caption']?>"> </picture> </figure> 

    And two generic suggestions:

    • short open tag is forbidden, always use <?php instead of <?
    • do not neglect the code indentation. The proper formatting is very important, it helps to understand what does the code do
    \$\endgroup\$
    6
    • \$\begingroup\$Thanks for your detailed feedback!\$\endgroup\$CommentedAug 1, 2022 at 16:56
    • 1
      \$\begingroup\$Just use your two nested loops but not to generate HTML, but to create a 1-level array. It should be quite the same.\$\endgroup\$CommentedAug 1, 2022 at 17:59
    • \$\begingroup\$Ping me If you don't manage, I'll get you a sketch\$\endgroup\$CommentedAug 2, 2022 at 3:47
    • 1
      \$\begingroup\$I already added it, not tested tho\$\endgroup\$CommentedAug 2, 2022 at 15:55
    • \$\begingroup\$Works great. I just had to change "<?php foreach ($image['srcset'] as $row): ?>", in our original answer ("end code"), to your $new array, "<?php foreach $new as $row): ?>. Thanks again. I learnt a lot from your answer.\$\endgroup\$CommentedAug 2, 2022 at 16:57

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.