View Single Post
08-24-13, 09:34 AM   #9
Vrul
A Scalebane Royal Guard
 
Vrul's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2007
Posts: 404
Originally Posted by Rainrider View Post
What is the difference between "needed in all iterations" and "needed in each independent iteration"? Please ignore the use of maxCPoints as the upper bound of the loop as this would be an obvious reason to define it locally in a higher scope as I also need it in each iteration. I thought that the point here would be that I assign a global (MAX_COMBO_POINTS) to maxCPoints, so that I would have a local variable definition, a global look-up and a garbage collect(??) for each loop iteration, where the global look-up is the most expensive part. So would the global look-up be reason enough to define maxCPoints outside the loop despite it being used only in the loop?
I think you misunderstood the message Phanx was trying to convey. She meant more along the lines of your cPoint definition inside the loop as being the correct way to do that as some people would define cPoint outside the loop and update it within the loop in the hopes of avoiding non-existent overhead from garbage collection.

A good rule of thumb is that the cost of defining a local vs indexing hits the break even point at 3 look-ups. So, if you need an indexed value 3+ times you are not hurting anything by creating a local reference and using it instead.

That said, you do not need MAX_COMBO_POINTS within the loop at all. The calculation you are doing is static for each iteration so it only needs to be done once prior to the loop. Doing that also lets you get rid of the cPoint:GetWidth() index/call as you will already have that value.
Code:
local AddComboPointsBar = function(self, width, height, spacing)
    local comboPoints, overlay, pointColors = {}, self.Overlay, ns.colors.cpoints
    width = (width - (MAX_COMBO_POINTS - 1) * spacing) / MAX_COMBO_POINTS
    spacing = width + spacing

    for i = 1, MAX_COMBO_POINTS do
        local cPoint = overlay:CreateTexture(nil, "OVERLAY")
        cPoint:SetSize(width, height)
        cPoint:SetPoint("BOTTOMLEFT", (i - 1) * spacing + 1, 1)
        local color = pointColors[i]
        cPoint:SetTexture(color[1], color[2], color[3], color[4])
        comboPoints[i] = cPoint
    end

    self.CPoints = comboPoints
end
Changes:
1. Moved the size calculations to outside the loop
2. Remove the name of the points to avoid clutter in _G
3. Changed the math a bit on sizing to make full use of the width passed
4. Used local references for indexes used in the loop (if you don't use the alpha color parameter remove the color[4] bit)

With that said, I can't imagine that function ever being called enough times to have ever warranted more that a quick pass for optimization. You only really need to scrutinize code for heavily called stuff like OnUpdate scripts and certain event handlers (CLEU, UNIT_AURA, etc...).
  Reply With Quote