Search code examples
objectooplualua-table

Lua Why can't Button:new run self.load()?


I would like to create buttons. Now I am doing it in the following way:

Button = {}
Button.__index = Button

function Button:new(x, y, text)  -- constructor
    self.__index = self
    self.x = x
    self.y = y
    self.text = text

    self.renderText = function()
        print(self.text)
    end
    
    self.load = function()
        if self.text ~= nil then
            self.renderText()
        end
    end

    self:load()
end

It is working properly, however, I would like to do it in the following way:

Button = {}

function Button:renderText()
    print(self.text)
end

function Button:load()
    if self.text ~= nil then
        self.renderText()
    end
end

function Button:new(x, text)  -- constructor
    local o = {}
    setmetatable(o, self)
    self.__index = self
    self.x = x
    self.text = text
    self:load()
    return o
end

The way I would like to do it does not work. I must run the self.load() function as soon as I create an instance of the class.


Solution

  • It is working properly

    No, it isn't. Your constructor is not constructing anything. Your metatable is unused - there is no call to setmetatable; whichever __indexes you install, they don't take any effect. Let's remove this "dead code". Let's also remove x and y for simplicity. This leaves us with:

    Button = {}
    function Button:new(text) -- constructor
        self.text = text
        self.renderText = function()
            print(self.text)
        end
        self.load = function()
            if self.text ~= nil then
                self.renderText()
            end
        end
        self:load()
    end
    

    Now recall that the definition function Button:new(text) is just shorthand for function Button.new(self, text) and that similarly, the call Button:new(text) is shorthand for Button.new(Button, text).

    This means that if you call this as Button:new("foo"), you are not constructing a new instance. Instead, you are operating on your one and only Button instance - self will be Button. After calling the "constructor", Button.text will have been set to "foo". This is pretty much a verbose singleton reinitialized / replaced with each constructor call. In particular, the following code will not work as expected:

    local foo = Button:new("foo") -- foo!
    local bar = Button:new("bar") -- bar! overwrites foo!
    foo.renderText() -- bar!
    assert(foo == bar) -- they are the same instance!
    assert(foo == Button) -- they are Button, even!
    

    you could "fix" this by passing an object to be initialized as self, but that would be rather dirty:

    local foo = Button.new({}, "foo") -- foo!
    local bar = Button.new({}, "bar") -- bar!
    foo.renderText() -- foo!
    bar.renderText() -- bar!
    assert(foo ~= bar) -- they aren't the same instance!
    assert(foo ~= Button) -- they aren't Button!
    

    Now, if you actually want a singleton, there's no need for metatables and the self syntactic sugar : - simply stick everything in a table:

    TheOneAndOnlyButton = {text = "foo"}
    function TheOneAndOnlyButton.renderText()
        print(TheOneAndOnlyButton.text)
    end
    function TheOneAndOnlyButton.load()
        if TheOneAndOnlyButton.text ~= nil then
            TheOneAndOnlyButton.renderText()
        end
    end
    TheOneAndOnlyButton.load()
    

    and here is how to properly do "improper" (metatable-less, closure-based) OOP where all closures have self as upvalue:

    function Button(text) -- constructor - note: we don't need `self` here
        local self = {text = text} -- actually create a new object
        self.renderText = function()
            print(self.text) -- self is available as an upvalue
        end
        self.load = function()
            if self.text ~= nil then
                self.renderText()
            end
        end
        self.load() -- we don't need to pass self, it's an upvalue
    end
    local foo = Button("foo")
    

    Why is this "improper"? Because all methods have to be instantiated again & stored in self for each self instance. This is suboptimal in terms of memory usage and also makes the constructor slower.

    Now finally onto the "proper" "OOP". The only thing that's "wrong" here is your constructor. The issue is simple: You're again mutating / indexing self, which will be Button when you call this as Button:new(...), rather than mutating / indexing o. Here's the fixed constructor (omitting X):

    function Button:new(text)  -- constructor
        local o = {}
        setmetatable(o, self)
        self.__index = self
        o.text = text -- o.text, not self.text! `self` is `Button`!
        o:load() -- o:load(), not `self:load()! `self` is `Button`!
        return o
    end
    

    This is exactly why I don't like the "Lua standard style" OOP as outlaid in "Programming in Lua": It is terribly error prone since everything is sticked into the same table - the methods, default values, the constructor, and metatable fields are all in one table (Button in this case). Here's an example of how you could cleanly separate everything into different tables / functions:

    local methods = {}
    local metatable = {__index = methods}
    function methods:renderText()
        print(self.text)
    end
    function methods:load()
        if self.text ~= nil then
            self:renderText()
        end
    end
    function Button(text) -- constructor
        local self = setmetatable({text = text}, metatable)
        self:load()
        return self
    end
    

    This eliminates the following mistakes:

    • Calling Class:method(...) rather than instance:method(...). This is exactly the mistake you were making here.
    • Calling the constructor as a method: instance:new(...)
    • Calling the constructor improperly: Class.new(...)