Most elegant way to iterate an array (Discussion type of question)

So I have a dilemma, which of the following two for loops is the prettier and better to use. The first is cleaner, whilst the second one is faster to type. But in the second one, I don’t like that I have to create a variable outside the for-each loop. Does this affect in any way the performance?

[Of course, I know a variable won’t affect the performance in any significant way, but if we imagine that this was a huge game, and I used this method hundreds of times, would it be garbage collected or it would catch a seat in memory and stay there?

Option 1#

for (int8 i = 0; i < Inventory.Num(); i++)
{
	if(Inventory[i] == nullptr)
	{
		Inventory[i] = NewObject<USlot>();
		Inventory[i]->SlotIndex = i;
	}
}

Option 2#

int8 i = 0;
for (auto Slot : Inventory)
{
	if(Slot == nullptr)
	{
		Slot = NewObject<USlot>();
		Slot->SlotIndex = i;
		i++;
	}
}

No doesn’t affect performance. In the 2nd case, it will get garbage collected once the function goes out of scope. The only drawback is you can’t change the size of the Inventory during a range-based for loop. You are really just doing the same thing as the first loop. The only difference is that the i automatically goes out of scope at the end of the for loop vs end of the function.
I would strongly urge only to fret over performance and memory allocation when it becomes a problem. If your fretting over declaring int’s there is probably other stuff going wrong. Int’s are ridiculously small in terms of memory allocation.

Don’t forget TArray::CreateIterator | Unreal Engine Documentation

	#include "Containers/Array.h"
	// ..
	for (auto It = MyArray.CreateIterator(); It; ++It)
	{
		It.GetIndex(); // returns current index of element.
		It.RemoveCurrent(); // Removes current
		It.Reset(); // Resets iterator to first element.
		It.SetToEnd(); // Sets iterator to end of array.
	}
8 Likes

Especially since this variable has automatic storage duration. The “allocation” is done when the function stack frame is created, so it is practically free.

As with everything, it depends.

Option 1 is probably better if you really need the index.
Option 2 is probably better if you can organize your inventory in a way to not require that index.

Option 2 also has a bug that if there are any nullptrs in your array, the index of the slot and the SlotIndex on your object will be mismatched. the i++ should be outside the nullptr check.

The iterator option is definitely interesting though it seems excessive to me just to deal with that index.

1 Like

It’s just a slot creating functionality in the constructor of the inventory, therefore everything is a nullptr at the beginning.

But literally speaking, I know that option two has flaws, that is the whole reason for this post. :slight_smile:

My lack of experience with C++… :sweat_smile:

Make sure you use auto& instead because right now you’re assigning a temporary value, the array element will remain null.