You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Nathan Marz (JIRA)" <ji...@apache.org> on 2009/01/30 22:18:59 UTC

[jira] Created: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

getFieldValue and setFieldValue should be abstract TBase methods
----------------------------------------------------------------

                 Key: THRIFT-297
                 URL: https://issues.apache.org/jira/browse/THRIFT-297
             Project: Thrift
          Issue Type: Improvement
          Components: Compiler (Java), Library (Java)
            Reporter: Nathan Marz


Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669816#action_12669816 ] 

Nathan Marz commented on THRIFT-297:
------------------------------------

+1

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>         Attachments: thrift-297.patch
>
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669045#action_12669045 ] 

Bryan Duxbury commented on THRIFT-297:
--------------------------------------

Yes, I can see how this would be really useful. However, there are some barriers. Currently, non-beans generated code doesn't contain the abstract getter and setter. This is because the internal implementation of those methods rely on beans-generated getters and setters for simplicity. As a result, we can't make getFieldValue and setFieldValue abstract on TBase, because non-beans code wouldn't meet that interface. 

We could do one of a few things. One approach would be to always generate the generic getters and setters, but in non-beans mode, just throw UnsupportedOperationExceptions. This would suck a bit, because then it's only a part-time interface. 

We could also always define the generic accesors, but have different code paths internally depending on beans and non-beans mode. This would be kind of ugly and verbose, so I don't really prefer this approach.

Another approach would be to always generate the beans-style getters and setters, whether or not we're in beans mode. That would essentially make beans/non-beans into private/public members, and that's it. This would let existing code continue to work the way it does, but allow us to extend the TBase interface nicely. 

Finally, an option I've suggested before to pretty tepid acceptance is doing away with non-beans altogether. I think a very valid argument could be made that non-beans generated code is at best a partial implementation of the Thrift functionality, and isn't the best face to put forward. While we're still pre-release, this would be a much easier change to make, and it would slim down the complexity of the Java code generator quite noticeably.

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Updated: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury updated THRIFT-297:
---------------------------------

    Patch Info: [Patch Available]

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>         Attachments: thrift-297.patch
>
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669089#action_12669089 ] 

David Reiss commented on THRIFT-297:
------------------------------------

The reason that the standard ("non-beans") interface exists is that we didn't want to mandate that application code use getters and setters for simple structures.  I think that it is a really good option and should not be eliminated.  Do you remember what the problem was with the generic getter/setter for the standard interface?  Is is just that you don't want to return "empty" values if the thing is unset?  I don't think there is a problem with that.  In the standard interface, we should take for granted that __isset might not be accurate, and just give the application code access to the data.

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669507#action_12669507 ] 

David Reiss commented on THRIFT-297:
------------------------------------

I don't see any downside.

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Resolved: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury resolved THRIFT-297.
----------------------------------

    Resolution: Fixed

I just committed this.

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>         Attachments: thrift-297.patch
>
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Updated: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury updated THRIFT-297:
---------------------------------

    Attachment: thrift-297.patch

I think this patch will do the trick.

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>         Attachments: thrift-297.patch
>
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Assigned: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury reassigned THRIFT-297:
------------------------------------

    Assignee: Bryan Duxbury

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669049#action_12669049 ] 

Nathan Marz commented on THRIFT-297:
------------------------------------

I agree that ideally we would get rid of non-beans generated code completely, and I would be completely for this. For the meantime, I think the proposal of "make beans/non-beans into private/public members, and that's it" is a great compromise and would seriously simplify these interface issues we're running into.

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669496#action_12669496 ] 

Bryan Duxbury commented on THRIFT-297:
--------------------------------------

Do you see any downside to always generating the setters and getters for fields? It certainly doesn't hurt to have the extra methods there if you don't want to use them. 

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>            Assignee: Bryan Duxbury
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669130#action_12669130 ] 

David Reiss commented on THRIFT-297:
------------------------------------

I think the sparseness is nice, but I don't think it is a critical feature.  (It was meant primarily to allow graceful addition of new fields.)  What code would be duplicated?  I'm looking at the "Simple getter" and "Simple setter" in generate_java_bean_boilerplate.  I don't think it would be necessary to automatically update __isset in the generic setter in non-beans code.

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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


[jira] Commented: (THRIFT-297) getFieldValue and setFieldValue should be abstract TBase methods

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-297?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12669095#action_12669095 ] 

Bryan Duxbury commented on THRIFT-297:
--------------------------------------

The problem with the standard interface is not so much the reading as it is the writing. Unless you do kind of a lot of extra work, you will find yourself writing a lot of "unset" primitive values to the wire. I view the sparseness of Thrift as a *key* feature, and am more than willing to pay the price of using accessor methods to maintain the __isset for me. 

Yes, we could recode the generic accessors to just return the raw data, ignoring the __isset. However, doing so would mean duplicating code with the beans-generated specific field accessors. 

> getFieldValue and setFieldValue should be abstract TBase methods
> ----------------------------------------------------------------
>
>                 Key: THRIFT-297
>                 URL: https://issues.apache.org/jira/browse/THRIFT-297
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Compiler (Java), Library (Java)
>            Reporter: Nathan Marz
>
> Since these methods can be called on every TBase instance, they should be part of the TBase API. These methods are useful in conjunction with the meta data features.

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