Displaying gallery when viewing category page or entry with related categories - code review

Question

I have a gallery in my site's <footer> with images that are defined in a matrix field on each of my categories. If I'm viewing a category page or an entry with related categories I want to display this image gallery in the footer.

What I currently have is this:

{% if category is defined %}
    {% if category.actionBilder.exists() %}
        <div id="footerslide">
            {% for block in category.actionBilder.limit(3).orderBy('RAND()').all() %}
                {% if block.type == "actionImg" %}
                    <div data-image="{{block.image.one().url}}"></div>
                {% endif %}
            {% endfor %}
        </div>
    {% endif %}
{% endif %}

{% if entry is defined %}
    {% set categories = craft.categories.relatedTo(entry) %}
    {% for category in categories %}
        {% if category.actionBilder.exists() %}
            <div id="footerslide">
                {% for block in category.actionBilder.limit(3).orderBy('RAND()').all() %}
                    {% if block.type == "actionImg" %}
                        <div data-image="{{block.image.one().url}}"></div>
                    {% endif %}
                {% endfor %}
            </div>
        {% endif %}
    {% endfor %}
{% endif %}

While it's working fine, I don't think that this is the best solution.

How would you change my code to make it better?

Answer 1

Some "problems" I see with your solution:

1. You're repeating yourself twice there - it would be much better if you had your actionBilder loop in there only once. When writing code, try to use the DRY principle: Don't Repeat Yourself (obviously exceptions to that rule but this is one case where you could simplify).
2. Not sure why exactly you're using exists() to check you matrix block's presence. I've actually never seen that method in use before and had to look it up to see what exactly it does. Based on the docs, looks like you would use it to check if there are any related categories (but don’t need to fetch them). But like I said, I've never heard of exists() so could be I'm wrong... Fwiw, here's a great answer by Brandon Kelly on what you should use to check if a field is empty or non-existent.
3. This may not be a problem in your current setup but at the moment you're fetching three blocks from your matrix field and only displaying them if they are a particular type. This means if you were to ever add another block to your matrix you could end up with two, one, or even no images at all (because the three blocks you queried for are all of a different type). It would be better to grab three blocks of that particular type and then loop through them.

Here's what I'd recommend doing:

{# are we dealing with an entry page? #}
{% if entry is defined %}

    {# if so we create an array of related categories #}
    {% set categories = craft.categories.relatedTo(entry) %}

{# otherwise we'll check if this is a category page #}
{% elseif category is defined %}

    {# if so we create an array with our one category #}
    {% set categories = [ category ] %}

{% endif %}

{# make sure we actually have categories in the array #}
{% if categories | length %}

    {# start the `for` loop #}
    {% for category in categories %}

        {# does the matrix field exist and does it contain any blocks? #}
        {% if category.actionBilder | length %}

            <div id="footerslide">

                {# get 3 `actionImg` blocks randomly #}
                {% for block in category.actionBilder.limit(3).type('actionImg').orderBy('RAND()').all() %}

                    {# get the block's first image's url #}
                    <div data-image="{{block.image.one().url}}"></div>

                {% endfor %}

            </div>

        {% endif %}

    {% endfor %}

{% endif %}

Hope that's helpful!