View Single Post
07-06-12, 05:56 PM   #2
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
You're lucky my browser restores the contents of form fields when I hit Back. Otherwise I'd be pretty pissed to have spent 20 minutes answering your questions only to be told that the thread did not exist after I hit Submit.

----------------------------------------------------------------------------------------------------------------------

(1) Declaring a variable inside a function vs. outside

(a)
Code:
local function myFunction()
	local name = GetSomeName()
	-- do other things
end
(b)
Code:
local name
local function myFunction()
	name = GetSomeName()
	-- do other things
end
Short answer:
I'd recommend (a), not (b), but there is no significant difference, so do what you want.

Long answer:
In terms of total memory/CPU usage, these are functionally identical. Both will create a new value (returned by the GetSomeName call) and create a pointer to it named "name" each time the function is run. What is different is when that value will be garbage-collected. In version (a) the value is discarded as soon as the function's execution finishes. In version (b) the value is held in memory until the next time the function is executed, and then discarded when the "name" pointer is redirected to the new value. From a code-readability perspective, I personally think (a) is cleaner and easier to read, too.

----------------------------------------------------------------------------------------------------------------------

(2) Defining a function inside another function

(a)
Code:
local function myFunction()
	myFrame:SetScript("OnUpdate", function()
		-- do things
	end)
	-- do other things
end
(b)
Code:
local function onUpdate()
	-- do things
end

local function myFunction()
	myFrame:SetScript("OnUpdate", onUpdate)
	-- do other things
end
Short answer:
Version (a) is bad. Don't do that. Functions are expensive to create. Do (b) instead.

Long answer:
There are a couple of specific situations where (a) is okay.

(i) You are only calling "myFunction" once the whole time your addon is loading and running. If you're calling it more than once, define the OnUpdate script handler function outside of "myFunction". If you're not sure whether you'll call it more than once, do (b) instead.

(ii) You are using "myFunction" to construct a custom function based on other variables that are also defined inside "myFunction". Many addons using AceConfig options tables do this. Here's an example from Grid that creates some dynamic options:

Code:
	for class, name in pairs(classes) do
		local class, name = class, name
		auraOptions.class.args[class] = {
			name = name,
			desc = string.format(L["Show on %s players."], name),
			type = "toggle",
			get = function()
					return GridStatusAuras.db.profile[status][class] ~= false
				end,
			set = function(_, v)
					GridStatusAuras.db.profile[status][class] = v
					GridStatusAuras:UpdateAllUnitAuras()
				end,
		}
	end
If you're not doing either of those things, you should use (a).

----------------------------------------------------------------------------------------------------------------------

(3) Function return values in for loop parameters

(a)
Code:
local myNumber = GetSomeNumberFromServer()
for i = 1, myNumber do
	-- do things
end
(b)
Code:
for i = 1, GetSomeNumberFromServer() do
	-- do things
end
No functional difference. "GetSomeNumberFromServer" will only be called once. This is clearly stated in the Lua manual section on for loops.

In general, I'd recommend (b) since it is less code to read. The only reason to do (a) would be if you need to use the same value more than once:

Code:
local n = GetNumRaidMembers()
if n > 0 then
	for i = 1, n do
		-- do things
	end
end
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote