Why is it okay to declare a STRUCT inside a nested loop?

Question

Here's a snippet of some supplied (CS50) code that declares the same STRUCT 'triple', over and over again inside of a nested loop. Why is this okay? In my mind, it would be more efficient to declare the STRUCT 'triple' just above the scope of the nested loops and change the values each iteration.

typedef struct
{
    BYTE rgbtBlue;
    BYTE rgbtGreen;
    BYTE rgbtRed;
} __attribute__((__packed__))
RGBTRIPLE;

.

 for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++)
{
    // iterate over pixels in scanline
    for (int j = 0; j < bi.biWidth; j++)
    {
        // temporary storage
        RGBTRIPLE triple;

        // read RGB triple from infile
        fread(&triple, sizeof(RGBTRIPLE), 1, inptr);

        // write RGB triple to outfile
        fwrite(&triple, sizeof(RGBTRIPLE), 1, outptr);
    }

Nagel

I think "assign" should be considered "declare" in this context - I don't expect an avowed beginner to know all the terminology :) . – Daven

Yes, my bad on the assign nomenclature, it's declared - I'll fix that. Still, are thousands of iterations of the same declaration is better than a single declaration right before the first loop? does this apply to all variable, it's okay to declare them over and over, within the scope they will used? – Nagel

A quick note of thanks to everyone who took the time to help a newbie. It's a long road from ignorance to understanding, and this is forum is a great vehicle for the long strange trip... -Bill – Nagel

Bill, glad to be able to help! Would you please hit the check mark next to whichever answer worked best for you to mark that answer as accepted? That will help future readers, and will give you and the answerer a reputation increase. Thanks! – Daven

Ahh, I didn't realize I needed to pick winner... If picking a line at the grocery store is an indicator, my choice is not going to be very helpful. In truth, much of getting it correctly was the interaction in the comments. Thanks, again! – Nagel

Thanks! Not a winner - many questions have accepted answers that aren't the top-voted answer. Anyway, happy programming! – Daven

Answer 1

Efficiency at this level is the compiler's concern in most cases. The compiler may well reuse the same stack space for each RGBTRIPLE! (Although it doesn't have to.)

Putting the RGBTRIPLE within the smallest brace pair (scope) that needs it prevents you from accidentally, erroneously accessing that variable outside that scope, when the variable's contents may not be valid.

Daven

Scope safety over efficiency is definitely the more important takeaway from this code snippet, especially for a beginner. +1 – Glosseme

Yes and I'd go even farther than @PhilipWrage to suggest that scope safety is the only takeaway here. The useful lesson for a beginner is that despite appearances, this declaration has no efficiency impact because by itself it doesn't actually generate any executable material. – Introject

Answer 2

Still, are thousands of iterations of the same declaration is better than a single declaration right before the first loop?

Sure this is all right. Either way, a good compiler will not emit code having any performance difference.

What might make for a modest linear performance change would be to call fread() less often.

for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++) {
    RGBTRIPLE triple[bi.biWidth];
    fread(triple, sizeof triple, 1, inptr);
    fwrite(triple, sizeof triple, 1, outptr);
}

or even

RGBTRIPLE triple[biHeight][bi.biWidth];
fread(triple, sizeof triple, 1, inptr);
fwrite(triple, sizeof triple, 1, outptr);

Lots of factors go into coding consideration. Avoid focusing on micro optimizations such as these.

Pinnati

Thank you all for the help; my takeaway is maintaining the smallest data scope necessary trumps looping through a variable declaration. This wasn’t about optimization per se, it’s my neuroses of needing to know how things work. Last question: A breakpoint between the declaration and fread lines; would have a valid placeholder for the elements of triple, but not valid data (regardless of iterations). The data in ‘triple’ from the prior iteration is gone at the point of re-declaration. True? RGBTRIPLE triple[bi.biWidth]; BREAK POINT fread(triple, sizeof triple, 1, inptr); – Nagel

@2Tall " The data in ‘triple’ from the prior iteration is gone at the point of re-declaration" --> Not quite. The object lifetime lasts until the end of the {} block. – Pinnati

So a declaration only has an impact on the executing code within the {} the first time it's encountered. You just need to declare it before use, and its gone when you're finished with the scope. That makes it easy keep things tight, thank you for the clarification - it's not the way I envisioned the process. – Nagel

Answer 3

What's important to understand here is that the statement RGBTRIPLE triple; declares the variable but does not directly correspond to "creating storage," or indeed translate to any machine language output at all.

You're just making a statement to the compiler about scope and usage of this variable (and declaring inside the most local block is a good-practice way of saying you only want it to be valid within that area). You could have put this line outside the loops, or at the top of its function, with no change to the executable output.

The compiler's job is to effectively create space on the stack for runtime use of local variables. In practice, it will simply reuse that same space used over and over again for each loop iteration. (People here will tell you, correctly, that this is not guaranteed, behavior which is technically true, but in practice it will always reuse the same space, as if you'd declared it above the loops.)

Introject