lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]


Tiago Katcipis wrote:
> http://github.com/katcipis/Luanotify
> Any suggestions will be very welcome :-), this is our FIRST Lua code, so
> i expect a lot of ugly and stupid things on it.

That's a nice, simple API.

I like the signal:stop() idea. I usually use return values to stop
further processing. Your way is more explicit.

A few observations...

Why did you choose the name 'add_tear_down'? That implies (to me,
anyway) that it will be invoked only when the signal object is
destroyed. I would have chosen something like 'add_post_emit'. A similar
comment can be made about 'add_set_up', of course.

You might want to use the set_up/tear_down functions themselves as keys
to the list so that you don't have to search for them (untested):

function Signal:add_set_up(set_up_func)
    if (type(set_up_func) ~= "function") then
        error("add_set_up: expected function...");
    end

    if not self.set_up_funcs[set_up_func] then
        self.set_up_funcs[#self.set_up_funcs+1] = set_up_func
        self.set_up_funcs[set_up_func] = #self.set_up_funcs
    end
end

function Signal:remove_set_up(set_up_func)
    local pos = self.set_up_funcs[set_up_func]
    if pos then
        table.remove(self.set_up_funcs, pos)
        self.set_up_funcs[set_up_func] = nil
    end
end

It might be better to run the tear_down functions in the opposite order
from the set_up functions, so they operate more like a stack. That way
the tear_down can undo any effects of its associated set_up for the next
(previous!) tear_down.

That make sense if you think of them as a push/pop or encode/decode
pair. If so, then it might be better to register the set_up and
tear_down functions as a pair (or add that as a separate call). But
think about the common use cases for set_up/tear_down.

As a final option, you might just want to let the signal itself define
set_up() and tear_down() and only call them if present:

function Signal:emit(...)
    self.signal_stopped = false;

    if self.set_up then
        self.set_up()
    end

    for _, handler_table in ipairs(self.handlers) do
        if(self.signal_stopped) then break end
        if(handler_table.block == 0) then
            handler_table.handler(...)
        end
    end

    if self.tear_down then
        self.tear_down()
    end
end

The down side is that there would be only one of each (do you really
need multiples? can't the object take care of chaining them if necessary?).

Doug


______________________________________________________________________________________
The information contained in this email transmission may contain proprietary and business 
sensitive information.  If you are not the intended recipient, you are hereby notified that 
any review, dissemination, distribution or duplication of this communication is strictly 
prohibited.  Unauthorized interception of this e-mail is a violation of law.  If you are not 
the intended recipient, please contact the sender by reply email and immediately destroy all 
copies of the original message.

Any technical data and/or information provided with or in this email may be subject to U.S. 
export controls law.  Export, diversion or disclosure contrary to U.S. law is prohibited.  
Such technical data or information is not to be exported from the U.S. or given to any foreign
person in the U.S. without prior written authorization of Elbit Systems of America and the 
appropriate U.S. Government agency.