[jira] Issue Comment Edited: (THRIFT-301) Turn read() into a class method

["Esteve Fernandez (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673840#action_12673840 ] 

esteve edited comment on THRIFT-301 at 2/16/09 2:59 AM:
------------------------------------------------------------------

So, what do you think of implementing this feature in a different method (e.g. create())? The read() method wouldn't go away and existing code would still work, but transports and protocols would use create(), instead of read() Actually, when deserializing a structure, they create an empty instance and call read() on it, so using create() is completely equivalent.

As for how to implement factories, here's some pseudopython for how I'd do it (feel free to point out anything you think is wrong :-)) :

class MyStructFactory(TStructFactory):

    def __init__(self, registry=None):
        if registry is None:
            registry = {}
        self.registry = registry
        self.registry[Foo] = self.createFoo

    def create(self, cls, **kwargs):
        try:
            return self.registry[cls](**kwargs)
        except KeyError:
            return cls(**kwargs)

    def createFoo(self, id=None, data=None):
        try:
            return self.fooPool[id]
        except KeyError:
            self.fooPool[id] = Foo(id=id, data=data)
            return self.fooPool[id]

class Foo:

    @classmethod
    def create(cls, iprot, structFactory=None):
        if structFactory is None:
            structFactory = TStructFactory()

        ...

        return structFactory.create(cls, **kwargs)

we could still make some more aggressive optimizations passing the protocol instance to the factory, so that it could skip some fields if it deems it appropriate (e.g. a Foo object with a particular id is already in the fooPool variable and thus we don't need to read the other members). Anyway I'd rather start by agreeing on the create() method and then file subsequent issues to implement the other features.

      was (Author: esteve):
    So, what do you think of implementing this feature in a different method (e.g. create())? The read() method wouldn't go away and existing code would still work, but transports and protocols would use create(), instead of read() Actually, when deserializing a structure, they create an empty instance and call read() on it, so using create() is completely equivalent.

As for how to implement factories, here's some pseudopython for how I'd do it (feel free to point out anything you think is wrong :-)) :

class MyStructFactory(TStructFactory):

    def __init__(self, registry=None):
        if registry is None:
            registry = {}
        self.registry = {}
        self.registry[Foo] = self.createFoo

    def create(self, cls, **kwargs):
        try:
            return self.registry[cls](**kwargs)
        except KeyError:
            return cls(**kwargs)

    def createFoo(self, id=None, data=None):
        try:
            return self.fooPool[id]
        except KeyError:
            self.fooPool[id] = Foo(id=id, data=data)
            return self.fooPool[id]

class Foo:

    @classmethod
    def create(cls, iprot, structFactory=None):
        if structFactory is None:
            structFactory = TStructFactory()

        ...

        return structFactory.create(cls, **kwargs)

we could still make some more aggressive optimizations passing the protocol instance to the factory, so that it could skip some fields if it deems it appropriate (e.g. a Foo object with a particular id is already in the fooPool variable and thus we don't need to read the other members). Anyway I'd rather start by agreeing on the create() method and then file subsequent issues to implement the other features.
  
> Turn read() into a class method
> -------------------------------
>
>                 Key: THRIFT-301
>                 URL: https://issues.apache.org/jira/browse/THRIFT-301
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Python), Library (Python)
>            Reporter: Esteve Fernandez
>         Attachments: thrift-301.patch, thrift-301_v2.patch, thrift-301_v3.patch, thrift-301_v4.patch, thrift-301_v5.patch, thrift-301_v6.patch
>
>
> Currently, the read() method is bound to each instance, but this makes impossible to make structures immutable (see THRIFT-162). The following patch changes the compiler to generate a class-bound read() method, it also updates the fastbinary extension accordingly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.