cleaning up script advice to use locals [Solved]

[pvklink]

Hi,

In one of the last topics i got the advise to use locals, so i am updating all my scripts...

1. do i also have to add
local colors and
local actions and what value do i have to give them by default?
2. does local controlVar = "controlVar" -- var, define as type string ...........have to be declared before ........return {
or can i put this rule with the rest of the local declarations.
3. is there a difference where to place the local declarations?

Code: Select all

-- maak variable controlVar Aan
-- dz.helpers.alertLidl(dz, "paars", 12) daarmee kun je t licht n kleur geven
-- let op ook scripts/dzVents/scripts/global_data.lua

local controlVar = "controlVar"  -- var, define as type string

return {
            on =    {   devices = { "usb lamp" }, -- controller , define as virtual selector Switch with all level as named in action table 
                        variables = { controlVar },   -- var, define as type string
                    },
                    
        logging =   {   level     =   domoticz.LOG_ERROR,
                        marker    =   "Lidl"   },

    execute = function(dz, item, info)
        local triggerDevice_Off    = dz.devices('$usb_lamp_uit')
        local triggerDevice_Aan    = dz.devices('$usb_lamp_aan')
        local triggerDevice_Kleur  = dz.devices('$usb_lamp_kleur')
        local triggerDevice_Kleuren= dz.devices('$usb_lamp_kleur')
        local triggerDevice_Wit    = dz.devices('$usb_lamp_wit')
        local triggerDevice_Flame  = dz.devices('$usb_lamp_flame')
        local triggerDevice_Play   = dz.devices('$usb_lamp_play')
        local triggerDevice_Dim    = dz.devices('$usb_lamp_dim')
        local triggerDevice_Speed  = dz.devices('$usb_lamp_speed')

        local logcode = 3
        local messageTable = {}
        local add = 'add'
        local del = 'del'
        local chg = 'chg'
        
        local function globalMessage(action, message, logcode)
 
            if logcode == nil then logcode = 3 end
 
            if action == add and logcode > 0 then
                messageTable = dz.helpers.addMessage(dz, item, info, message, messageTable, logcode)
            elseif action == del then
                dz.globalData.mylogging = message
                dz.devices('timerlog').updateText(message)
            elseif action == chg then 
                dz.helpers.dumpMessages(dz, messageTable)
            end
 
        end
    
        local controlVar = dz.variables(controlVar)
        
        colors = { rood = 1,
                   groen = 2,
                   blauw = 3,
                   oranje = 4, 
                   grijs = 5,
                   roze = 6,
                   paars = 7,
                 }
        
        actions =   {   Off = function() triggerDevice_Off.switchOn() return end,
                        Aan = function() triggerDevice_Aan.switchOn() triggerDevice_Flame.switchOn() return end,
                        Kleuren = function() triggerDevice_Aan.switchOn() triggerDevice_Kleuren.switchOn() return end,
                        Kleur = function(color) 
                                    triggerDevice_Off.switchOn() 
                                    triggerDevice_Aan.switchOn() 
                                    local repeats = (( colors[color] or 1 ) - 1)
                                    dz.log("Color requested :" .. tostring(color) .. "==>> repeats required " .. repeats ,dz.LOG_DEBUG )
                                    triggerDevice_Kleur.switchOn().repeatAfterSec(0.1, repeats )
                                    return 
                                end,
                        Wit = function() triggerDevice_Aan.switchOn() triggerDevice_Wit.switchOn() return end,
                        Flame = function() triggerDevice_Aan.switchOn() triggerDevice_Flame.switchOn() return end,
                        Play = function() triggerDevice_Aan.switchOn() triggerDevice_Play.switchOn() return end,
                        Dim = function() triggerDevice_Aan.switchOn() triggerDevice_Dim.switchOn() return end,
                        Snel = function() triggerDevice_Aan.switchOn() triggerDevice_Speed.switchOn() return end, 
                    }
        
        local function setVar( request )
            if controlVar.value ~= request then
                controlVar.set(request).silent()
            end
        end
        
        local function isColorAction( action )
            for color, number in pairs(colors) do -- check if called with one of the defined colors
                if action == color then

                    globalMessage(add, ' USB LAMP: krijgt waarde: ' .. action,logcode) 

                    setVar(color)
                    actions["Kleur"](color)
                    return true
                end
            end
            return false
        end
        
        local function doAction( action )
            if actions[action] then

                globalMessage(add, ' USB LAMP: krijgt waarde: ' .. action,logcode) 

                setVar(action)
                actions[action]()    -- Call required function with key = action
            else
                globalMessage(add, ' USB LAMP: heeft een onbekende waarde: ' .. action,logcode) 

                return false
            end
        end
        
        if item.isDevice and ( not ( isColorAction(item.levelName) ) ) then 
            globalMessage(add, ' USB LAMP: krijgt waarde: ' .. item.levelName,0) 

            doAction(item.levelName) 
        elseif item.isVariable and ( not ( isColorAction(item.value) ) ) then 
            globalMessage(add, ' USB LAMP: krijgt waarde: ' .. item.value,0) 

            doAction(item.value) 
        end

        globalMessage(chg) -- dump
    end
}

[jake]

I simply declare it as: local controlVar

You can do it on top of the script or below the start of the dzvents party of the script

[waaren]

I got the advise to use locals, so i am updating all my scripts...

1. do i also have to add local colors and local actions and what value do i have to give them by default?

It is not so much a matter of 'have to'. It is considered good practice to limit the scope of variables (in Lua that is also including functions and tables) to where they are needed.

• It is better for performance (local vars in Lua are in general 20%-30% faster then global ones)
• They don't pollute the global space (global vars are stored in the _G global table)
• It can prevent easy to make errors and your code is easier to debug.

for howto change colors and actions to local tables see code below.

2. does local controlVar = "controlVar" -- var, define as type string ...........have to be declared before ........return {
or can i put this rule with the rest of the local declarations.

it is used for the first time in the on = section at line 9 so it has to be declared before that line

3. is there a difference where to place the local declarations?

No it's a matter of personal preference about readability but because you want to limit the scope of the local variables to the code-block where it is used it is best to place them in that block.
So if only used in a function; within that function
if only used in a repeat-until block; within that block
etc..

Code: Select all

-- maak variable controlVar Aan
-- dz.helpers.alertLidl(dz, "paars", 12) daarmee kun je t licht n kleur geven
-- let op ook scripts/dzVents/scripts/global_data.lua

local controlVar = "controlVar"  -- var, define as type string

return {
            on =    {   devices = { "usb lamp" }, -- controller , define as virtual selector Switch with all level as named in action table 
                        variables = { controlVar },   -- var, define as type string
                    },
                    
        logging =   {   level     =   domoticz.LOG_ERROR,
                        marker    =   "Lidl"   },

    execute = function(dz, item, info)
        local triggerDevice_Off    = dz.devices('$usb_lamp_uit')
        local triggerDevice_Aan    = dz.devices('$usb_lamp_aan')
        local triggerDevice_Kleur  = dz.devices('$usb_lamp_kleur')
        local triggerDevice_Kleuren= dz.devices('$usb_lamp_kleur')
        local triggerDevice_Wit    = dz.devices('$usb_lamp_wit')
        local triggerDevice_Flame  = dz.devices('$usb_lamp_flame')
        local triggerDevice_Play   = dz.devices('$usb_lamp_play')
        local triggerDevice_Dim    = dz.devices('$usb_lamp_dim')
        local triggerDevice_Speed  = dz.devices('$usb_lamp_speed')

        local logcode = 3
        local messageTable = {}
        local add = 'add'
        local del = 'del'
        local chg = 'chg'
        
        local function globalMessage(action, message, logcode)
 
            if logcode == nil then logcode = 3 end
 
            if action == add and logcode > 0 then
                messageTable = dz.helpers.addMessage(dz, item, info, message, messageTable, logcode)
            elseif action == del then
                dz.globalData.mylogging = message
                dz.devices('timerlog').updateText(message)
            elseif action == chg then 
                dz.helpers.dumpMessages(dz, messageTable)
            end
 
        end
    
        local controlVar = dz.variables(controlVar)
        
        local colors = { rood = 1,
                   groen = 2,
                   blauw = 3,
                   oranje = 4, 
                   grijs = 5,
                   roze = 6,
                   paars = 7,
                 }
        
        local actions =   {   Off = function() triggerDevice_Off.switchOn() return end,
                        Aan = function() triggerDevice_Aan.switchOn() triggerDevice_Flame.switchOn() return end,
                        Kleuren = function() triggerDevice_Aan.switchOn() triggerDevice_Kleuren.switchOn() return end,
                        Kleur = function(color) 
                                    triggerDevice_Off.switchOn() 
                                    triggerDevice_Aan.switchOn() 
                                    local repeats = (( colors[color] or 1 ) - 1)
                                    dz.log("Color requested :" .. tostring(color) .. "==>> repeats required " .. repeats ,dz.LOG_DEBUG )
                                    triggerDevice_Kleur.switchOn().repeatAfterSec(0.1, repeats )
                                    return 
                                end,
                        Wit = function() triggerDevice_Aan.switchOn() triggerDevice_Wit.switchOn() return end,
                        Flame = function() triggerDevice_Aan.switchOn() triggerDevice_Flame.switchOn() return end,
                        Play = function() triggerDevice_Aan.switchOn() triggerDevice_Play.switchOn() return end,
                        Dim = function() triggerDevice_Aan.switchOn() triggerDevice_Dim.switchOn() return end,
                        Snel = function() triggerDevice_Aan.switchOn() triggerDevice_Speed.switchOn() return end, 
                    }
        
        local function setVar( request )
            if controlVar.value ~= request then
                controlVar.set(request).silent()
            end
        end
        
        local function isColorAction( action )
            for color, number in pairs(colors) do -- check if called with one of the defined colors
                if action == color then

                    globalMessage(add, ' USB LAMP: krijgt waarde: ' .. action,logcode) 

                    setVar(color)
                    actions["Kleur"](color)
                    return true
                end
            end
            return false
        end
        
        local function doAction( action )
            if actions[action] then

                globalMessage(add, ' USB LAMP: krijgt waarde: ' .. action,logcode) 

                setVar(action)
                actions[action]()    -- Call required function with key = action
            else
                globalMessage(add, ' USB LAMP: heeft een onbekende waarde: ' .. action,logcode) 

                return false
            end
        end
        
        if item.isDevice and ( not ( isColorAction(item.levelName) ) ) then 
            globalMessage(add, ' USB LAMP: krijgt waarde: ' .. item.levelName,0) 

            doAction(item.levelName) 
        elseif item.isVariable and ( not ( isColorAction(item.value) ) ) then 
            globalMessage(add, ' USB LAMP: krijgt waarde: ' .. item.value,0) 

            doAction(item.value) 
        end

        globalMessage(chg) -- dump
    end
}

[waaren]

I simply declare it as: local controlVar

You can do it on top of the script or below the start of the dzvents party of the script

Sorry but you missed the use of it in line 9 .

[pvklink]

@Thanks.... i like good practices from pro's.....
i am a nono with programming so ....
and yes! i missed the use in line 9, stupid question