kikito / middleclass

Object-orientation for Lua
https://github.com/kikito/middleclass
MIT License
1.77k stars 190 forks source link

Indexable mixin doesn't work with 2.0 #14

Closed six8 closed 12 years ago

six8 commented 12 years ago

The middleclass-extras repo clearly says not to use it anymore but how do we do what Indexable allows you to do in 2.0?

kikito commented 12 years ago

Hello mike,

Indeed, indexable doesn't work with middleclass 2.0. I've been thinking about it for a while. I think it was too complex, and exploring other possibilities. Right now my best candidate involves creating an "impostor" for each instance; the code would do something similar to this:

local indexedMethods = {
  foo = function() ... end,
  bar = function() ... end
}

local MyClass = class('MyClass')
local prevAllocate = MyClass.allocate
function MyClass:allocate()
  local instance = prevAllocate(MyClass)
  local impostor = setmetatable({}, {
    __newindex = instance,
    __index = function(imp, name)
      return rawget(instance, name) or indexedMethods[name] or instance[name]
    end
  })
  return impostor
end

local peter = MyClass:new() -- peter is not a "real" instance, but an "impostor"
peter:foo() -- the impostor knows methods that the real instance doesn't

That's basically what I think will happen under the hood. I still haven't thought about how to implement it as a mixin, and I anticipate some problems with the metatables - probably I'll have to include a __metatable metamethod in the impostor.

So, I've thought about it, but I don't have anything finished, I'm afraid.

What do you need Indexable for, anyway? Maybe there is a way around it.

six8 commented 12 years ago

I'm wrapping an object that's been created in C so I can't modify it. But I want my wrapper class to look and function like the object I'm wrapping. I did this in middleclass 1.4 with the Indexable mixin. Handling methods is easy, I just set a reference to them on my wrapper class. However the properties need to be readable and writeable as well. The only way I could do that was something like this:

function Wrapper:index(name)
    local pk = wrappedKeys[name]
    if pk then
        return self.__wrapped_obj__[pk]
    else
        return nil
    end
end

I think a good solution to my problem, and probably for most needs of Indexable would to make it so you have getters and setters. So if you have:

function Wrapper.setters:foo(value)
    self.__wrapped_obj__.foo = value
end    

function Wrapper.getters:foo()
    return self.__wrapped_obj__.foo
end    

print(myWrapper.foo) -- Outputs self.__wrapped_obj__.foo

That way I could easily control proxying properties to the wrapped obj. I've seen this before, I just don't understand lua metatables enough to figure out how to mix it with middleclass.

kikito commented 12 years ago

Hi six8,

If I've understood your code correctly the table wrappedKeys holds a list with all the "C" functions. If that table is available before you have to create its first instance, I think it'll be faster and simpler to just define the functions in the class using a loop over wrappedKeys, like so:

-- example values
local wrappedKeys = { foo = 1 }
local wrapped = { foo = function() print('C function!') end } -- foo in reality will be C

Wrapper = class('Wrapper')

function Wrapper:initialize(obj)
  self.__wrapped_obj__ = obj
end

-- this is where the money is!
for methodName,_ in pairs(wrappedKeys) do
  Wrapper[methodName] = function(self, ...)
    -- depending on your case, you may want to remove the first "self" parameter here
    return self.__wrapped_obj__[methodName](self, ...) 
  end
end

obj = Wrapper:new(wrapped)
obj:foo() -- C function! 

Let me know what you think. Regards!

six8 commented 12 years ago

Sorry, for the confusion. wrappedKeys is simply the properties of the underlying object. I'm already doing what you suggest with methods. I need a way to be able to read and write properties from and to the underlying object. Such as:

obj = Wrapper:new(wrapped)
print(obj.foo) -- Value of property `foo` of wrapped
obj.foo = 1
print(obj.foo) -- Value of property `foo` of wrapped which is now 1

Thanks for your help!

kikito commented 12 years ago

Hi there,

Unless you need something else, setting obj.foo as you told me would work - obj.foo will indeed be 1. wrapped.foo will be unaltered, however.

If you are going to use just obj, then the code above should suffice. Otherwise it's going to be a heck more complicated - you will need to do more stuff in C, I think.

kikito commented 12 years ago

Hi again,

Another question - Do you actually define any methods at all in Wrapper, or is it just being used to access the wrapped object?

six8 commented 12 years ago

The point is that foo must be altered in the wrapped object. Also, the wrapped object's foo can be changed by other forces outside my wrapper class so that when you read wrapperObj.foo, it must reflect the current value of the wrapped object's foo. I'm currently doing this just fine by manipulating __index and __newindex with the Indexable mixin, but it would be cleaner to adopt the getter/setters like in http://nova-fusion.com/2011/04/04/implementing-proper-gettersetters-in-lua/ or http://lua-users.org/wiki/ObjectProperties. It could be cleaner if this was built in to middleclass as I think it's a common problem and would obviate the need for Indexable.

I am adding methods and properties to the wrapper.

kikito commented 12 years ago

"It could be cleaner if this was built in to middleclass as I think it's a common problem and would obviate the need for Indexable"

I think newindex is supported by middleclass. index is not - for performance reasons. Right now the attribute lookup is already slower than I would like. Adding the possibility of checking an index would slow every access to every object attribute, even those that don't need index (which are the majority of people).

Do you have a more high-level description of what you want to do? What are Wrapper and what is the wrapped object, really?

six8 commented 12 years ago

The wrapped object is specifically a Corona SDK display group. What I'm currently doing with Indexable is making a middleclass mixin that creates a display group and makes whatever class is using it act like Corona's display group. This allows me to use middleclass classes that I can extend and re-use as Corona display objects. My existing implementation is here

kikito commented 12 years ago

"My existing implementation is here"

woah! That's a lot of stuff! Very insteresting.

I see what you are trying to do better now.

Indexable would have only resolved one half of the problem - it would only help with the index part (does nothing with newindex). But the way you are handling that now is more efficient anyway; only the methods "redirected to the corona object" are affected by it. Using any kind of __index would make all the methods, even those not redirected, slower.

I'm not sure about what you want to accomplish with this schema though. I like small, specialized classes. "raw" displaygroups already do a lot of stuff. I'm not sure allowing people to add even more stuff to them encourages a good design. On this case, I'd try to use composition more than inheritance.