How could I make this for loop more efficient?

Hello! I use a fair bit of for loops in my scripts when I want to handle many objects of the same type.

Here's an example - Compares names in a table to object names and sets values based on what the values are equal to in a table
for i, v in pairs(bseClone:GetChildren()) do
		if mTable[v.Name] ~= nil then
			v.Value = mTable[v.Name]
		end
	end

However, I notice there’s a lot of other table based operations and ways to do for loops.
So my question is, what could be better about this for loop? I know this particular one isn’t too bad, but I have had some create lag in the past with bigger amounts of objects so I want to know more for those.

Cheers! :slight_smile:

Well I guess what you can do instead is, use the or operator in the variable.
So pretty much instead of having a for loop do this

for i, v in pairs(bseClone:GetChildren()) do
	v.Value = mTable[v.Name] or v.Value
end

The logic behind this is; whenever you set a variable to either two values using the or operator like this

local var = 4 or 5

you might be asking to what value is this going to be set to? 4 or 5? welp if the two values are littearly anything that is not false or nil, it will will always choose the first one, so in our 4 or 5 case, we have two number values so it will just choose the first one which is 4.
But if you were to have a false or nil in this it will always choose the other value

local var = nil or "hi"
print(var) -- prints hi

local var = "cool" or 10
print(var) -- prints cool

local var = false or true 
print(var) -- prints true

local var = false or 45
print(var)  --prints 45

local var = nil or 45
print(var) -- prints 45

local var = false or nil
print(var)
--this one is kind of weird because it always chooses the second value
--so it prints nil

local var = nil or false
print(var) --prints false

So yeah, so here is what’s going on with the script.

for i, v in pairs(bseClone:GetChildren()) do
	v.Value = mTable[v.Name] or v.Value
end

If mTable[v.Name] did exist within mTable so v.Value will be set to the first value, all good, but if mTablr[v.Name] didn’t actually exist, which mean it will be nil, since it is nil v.Value will be set to the other value which is just setting it back to itself so yeah I hope you understood.

I don’t really think there is anything else you can do to make this efficient, you have to use a for loop there is no way out.
Here we just replace 3 lines with 1 line which wouldn’t reduce the lag but would make it cooler looking

3 Likes

I didn’t know you could do this. It’s pretty wild, thanks! I’m more doing this to better organize my code and make it easier to work with in the future while trying to better iterate through possibly hundreds of objects. Even if it just makes my code look nicer, I appreciate it!

2 Likes

This is what #development-support:code-review is for.

Honestly, you’re not going to get anything more efficient than this. Is this loop causing noticeable delays or are you running it frequently enough to cause concern? Trying to make this “more efficient” than it already is is heading into microoptimisation grounds.

1 Like

ah of course

Will post there next time if I have a question like this.

And no, it’s for other loops that may need to handle lots of things which COULD cause lag (have in the past).

Every iteration of the loop you’re calling :GetChildren(). In this case you can call it once and save to a variable, then put the variable in the loop. Will it make a noticeable difference? Probably not. More efficient? Sure

1 Like