Peer Review, dynamically creating strings

[Cladhaire]

What I'm doing here is taking a string like this:

[name][leader] [pvp] [status] [curhp]/[maxhp] and parsing it out, and creating a closure that can be run at any point in the game and give current values for my unit frames. I've settled on a solution which looks like this:

function getFormatFunc(text)
	local output = {}
	local start = 1
	local last
	
	for s,data,e in string.gfind(text, "()(%b[])()") do
	
		if s > start then
			table.insert(output, string.sub(text, start, s - 1))
		end
		local tag = string.sub(data, 2, -2)
		if not UnitInformation[tag] then
			table.insert(output, function() return "" end)
		else
			table.insert(output, UnitInformation[tag])
		end
		
		start = e
		last = e
	end
	
	if string.len(text) > last then
		table.insert(output, string.sub(text, last, string.len(text)))
	end
	
	return function(unit)
		for k,v in pairs(output) do
			if type(v) == "function" then
				output[k] = v(unit)
			end
		end
		
		return table.concat(output)
	end
end

profile("Anon Format", getFormatFunc("[name][level] [pvp] [status] [curhp]/[maxhp]"))

And my results are reasonably promising.. I'm just wondering if you see anything that's a horrible decision, or ways I can tune this a little more (this was a last resort when I didn't get what I wanted with other approaches).

Feedback on the forums, or via [email protected] is appreciated.

[Iriel]

I'm not sure what this buys you since it only evaluates the functions once, and use of dummy functions returning an empty string instead of just an empty string looks like an unfinished thought.

I'd expect an optimization would be to avoid table.concat, then have your function generate the following for the closure (I'm assuming that the only-evaluates-once behaviour is an accident, and not intentional)

1: A string.format pattern with all of the 'static' elements in.
2: An array with the functions for each of the 'dynamic' elements
3: A temporary array (to avoid GC later)

Your returned function would then essentially be

for i,f in ipairs(dynamicElements)
  workArray[i] = f();
end
return string.format(formatString, unpack(workArray));

[Cladhaire]

Yep, I saw most of what you're talking about in the midst of this mess of code (going back and forth between work and looking at this, and trying different approaches). I caught the one-time evaluation a bit after I posted, but I just now got back to the forums.

If I get something more to post, I'll drop it here--- unless I get home first. Thanks for the feedback.

[Cladhaire]

For this approach, I'd like to use a single workArray to save on overall memory use (each unit frame could potentially have up to six of these functions, depending on how complex and efficient I'd like to get on the changing of elements.

I thought about doing that with a table.setn but I don't see gaining that much from that. Leaving that out only restricts the users to not using any of the string.format % characters in their format string, something which shouldn't be that much of a restriction (They'd throw an error either way), but my concern was taking a format string with 6 tags, and then using the same work table for a string with 4 tags-- you could pick up the info at the end of the array... but that doesn't matter in this case.

local workArray = {}

function getFormatFunc(text)
	local first, last = 1
	local formatArgs, formatString = {}
	
	for s,data,e in string.gfind(text, "()(%b[])()") do
		tag = string.sub(data, 2, -2)
		
		if UnitInformation[tag] then
			table.insert(formatArgs, UnitInformation[tag])
		end
	end

	formatString = string.gsub(text, "%b[]", "%%s")

	return function(unit)
		for i,f in ipairs(formatArgs) do
			workArray[i] = f()
		end
		return string.format(formatString, unpack(workArray))
	end
end

Thanks for helping me take a look at it.. I needed a fresh set of eyes =)

[Cladhaire]

Wanted to add a few features, so I recoded some of the middle logic:

function getFormatFunc(text)
	local first, last = 1
	local formatArgs, formatString = {}
	
	for s,data,e in string.gfind(text, "()(%b[])()") do
		tag = string.sub(data, 2, -2)
		
		if UnitInformation[tag] then
			table.insert(formatArgs, UnitInformation[tag])
		elseif string.find(tag, "%a+ %d+") then
			local _,_,tag,width = string.find(tag, "(%a+) (%d+)")
			table.insert(formatArgs, function(unit) if UnitInformation[tag](unit) then return string.sub(UnitInformation[tag](unit), 1, width) end end)
		elseif string.find(tag, "%a+ %p+") then
			local _,_,tag,oc,ec = string.find(tag, "(%a+) (%p)(%p)")
			table.insert(formatArgs, function(unit) if UnitInformation[tag](unit) ~= "" then return oc..UnitInformation[tag](unit)..ec else return "" end end)
		else
			self:error(data.." is not a valid format tag.")
		end
	end

	formatString = string.gsub(text, "%b[]", "%%s")

	return function(unit)
		for i,f in ipairs(formatArgs) do
			workArray[i] = f()
		end
		return string.format(formatString, unpack(workArray))
	end
end

Now it will allow tags like:

[name 5] which will truncate the string at five characters.
[pvp ()] which will wrap the return from PvP (if it exists) in parenthesis

I've got to play with a lot more, but I'm happy with where I'm getting =)

[Iriel]

Some minor changes:

1) Initializing workArray
2) Passing unit to the formatters at runtime

function getFormatFunc(text)
	local first, last = 1
	local formatArgs, workArray, formatString = {},{}
	
	for s,data,e in string.gfind(text, "()(%b[])()") do
		tag = string.sub(data, 2, -2)
		
		if UnitInformation[tag] then
			table.insert(formatArgs, UnitInformation[tag])
		elseif string.find(tag, "%a+ %d+") then
			local _,_,tag,width = string.find(tag, "(%a+) (%d+)")
			table.insert(formatArgs, function(unit) if UnitInformation[tag](unit) then return string.sub(UnitInformation[tag](unit), 1, width) end end)
		elseif string.find(tag, "%a+ %p+") then
			local _,_,tag,oc,ec = string.find(tag, "(%a+) (%p)(%p)")
			table.insert(formatArgs, function(unit) if UnitInformation[tag](unit) ~= "" then return oc..UnitInformation[tag](unit)..ec else return "" end end)
		else
			self:error(data.." is not a valid format tag.")
		end
	end

	formatString = string.gsub(text, "%b[]", "%%s")

	return function(unit)
		for i,f in ipairs(formatArgs) do
			workArray[i] = f(unit)
		end
		return string.format(formatString, unpack(workArray))
	end
end

You might find it more efficient to use a pair of format arrays:

formatTags -- Tag names
formatFunctions -- Formatting functions

and then do:

for i,f in ipairs(formatFunctions) do
  workArray[i] = f(unit, formatTags[i]);
end

That way you can instantiate a single 'truncate at 5 characters' function, rather than one for every field you use. Or a single 'optionally surround with parens' function, etc.

[Iriel]

The 2 table approach may be considered too complex, in which case I'd recommend you put the code that creates your fetch-and-format functions into subfunctions and cache them, so you have a cache for each of your subformat strings, so the first time you see "pvp ()" you'd create it on demand, and then store it in the cache:

formatCache["pvp ()"] = newFunction;

Then next time around, you can just grab it out of the format cache instead of having to make yet another function. As long as all of your functions take unit as their parameter then this will work pretty damn well.