Does this logic look right?

The code you’ve got written will not even compile – it isn’t legitimate C#. You do not want a stranger to inform you it is unsuitable when your compiler will do it immediately.

I believe what you need is one thing extra like this:

// Create a VARIABLE that may HOLD an array (no array is created on this line).
GameObject[] crates;

// PascalCase for methodology names. No return worth or parameter.
public void GetAllCrates() 
{
    // This line creates an array and assigns it to the variable we ready.
    crates = GameObject.FindGameObjectsWithTag("crate");
}

Adding x and making an attempt to repeatedly discover one crate at a time in a loop is simply extra complication.

If you’ll be able to, I’d advocate not utilizing tags for this. You’ve seemingly received a script on the crate objects anyway – possibly even one known as Crate. That would allow you to do that as an alternative:

Crate[] crates;

public void GetAllCrates() {
    crates = FindObjectsOfType<Crate>();
}

This could be extra environment friendly than string-matching tags, particularly should you later need to entry properties or strategies on the Crate script from gadgets within the array.