Input requested on my first addon -> FuBar - CloakHelmToggle

[brotherhobbes]

If anyone has some spare time and wouldn't mind looking through my code, I'd greatly appreciate it. Comments on the design and functionality of the addon would be nice, but some constructive criticism on what I did wrong would help me the most.

FuBar - CloakHelmToggle
http://www.wowinterface.com/download...fo.php?id=4816

Thanks for your time!

[TheFly]

while I assume it's fully functional I would probably have gone with less if statements.
UpdateText:
local HColor = ShowingHelm() and COLOR_HEX_GREEN or COLOR_HEX_RED
local cColor = ShowingVest() and COLOR_HEX_GREEN or COLOR_HEX_RED
self:SetText(FuBarUtils.Colorize(HColor, self.textCloak)..FuBarUtils.Colorize(CColor, self.textHelm))

UpdateTooltip
local IsCloak,IsHelm = ShowingCloak(),ShowingHelm()
FuBarTooltip:AddDoubleLine(nil, "Cloak", IsCloak and "On" or "Off", 1, 1, 0, IsCloak and 0 or 1, IsCloak and 1 or 0, 0)
FuBarTooltip:AddDoubleLine(nil, "Helm", IsHelm and "On" or "Off", 1, 1, 0, IsHelmk and 0 or 1, IsHelm and 1 or 0, 0)
FuBarTooltip:SetHint("Click to toggle.")

OnClick:
local i = (2*(ShowingCloak() or 0) + ShowingHelm() or 0 + 1)
if i>3 then i = i-4 end
if i>=2 then ShowCloak(1); i = i-2; else ShowCloak(0); end
ShowCloak(i)

My smart Idea of using modulus became less smart when I realized lua don't have modulus. I'm not sure I recommend this exact coding but it can clearly be done more consice than currently.

[brotherhobbes]

Thanks a bunch, TheFly.

I was able to change UpdateText to the following:
local HColor = ShowingHelm() and FuBarUtils.COLOR_HEX_GREEN or FuBarUtils.COLOR_HEX_RED
local CColor = ShowingCloak() and FuBarUtils.COLOR_HEX_GREEN or FuBarUtils.COLOR_HEX_RED
self:SetText(FuBarUtils.Colorize(HColor, self.textCloak)..FuBarUtils.Colorize(CColor, self.textHelm))

UpdateTooltip worked fine as listed.

The OnClick code is not working, though. ShowingCloak() returns 1 or nil and the nil is causing "attempt to perform arthimetic on a nil value". I modified the code to the following, but it doesn't quite work right. Most likely because I'm failing to understand a piece of it:
local ctest,htest
if ShowingCloak() then ctest = 1 else ctest = 0 end
if ShowingHelm() then htest = 1 else htest = 0 end
local i = (2 * (ctest or 0) + htest or 0 + 1)
if i > 3 then i = i - 4 end
if i >= 2 then ShowCloak(1); i = i - 2; else ShowCloak(0); end
ShowCloak(i)

Also, does self:Update() need to be called at the end of the OnClick()? That's more a general question, because I honestly do not know.