You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Bryan Duxbury (JIRA)" <ji...@apache.org> on 2009/11/07 19:43:32 UTC
[jira] Created: (THRIFT-623) Use a Java enum to represent field ids
in generated structs
Use a Java enum to represent field ids in generated structs
-----------------------------------------------------------
Key: THRIFT-623
URL: https://issues.apache.org/jira/browse/THRIFT-623
Project: Thrift
Issue Type: Improvement
Components: Compiler (Java)
Reporter: Bryan Duxbury
Assignee: Bryan Duxbury
Fix For: 0.2
Attachments: thrift-623.patch
The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-623) Use a Java enum to represent field ids
in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-623:
---------------------------------
Attachment: thrift-623.patch
Here's my first stab at this. The tests all pass.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12774659#action_12774659 ]
Todd Lipcon commented on THRIFT-623:
------------------------------------
looks good to me. Are there any tests for what happens when bad field IDs come through? (ie do the tests have coverage on the uncommon cases?)
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12774681#action_12774681 ]
Bryan Duxbury commented on THRIFT-623:
--------------------------------------
The tests do cover these cases, since I had to fix them when I ran the suite :)
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776727#action_12776727 ]
Bryan Duxbury commented on THRIFT-623:
--------------------------------------
There's nothing guaranteeing that someone won't make a field called "Fields_", is there? We're just trying to make this field some measured amount of unlikely so that people won't collide with it. Otherwise, reserved words are the way to go.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776735#action_12776735 ]
David Reiss commented on THRIFT-623:
------------------------------------
I guess I'd prefer a blanket rule that Thrift identifiers can't start with underscores and reserve all of those for internal use. It just keeps things more organized.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (THRIFT-623) Use a Java enum to represent field ids
in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury closed THRIFT-623.
--------------------------------
Resolution: Fixed
I just committed this.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623-v3.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12775260#action_12775260 ]
Bryan Duxbury commented on THRIFT-623:
--------------------------------------
I'm a bit loathe to add extra characters into a class name that I expect to make a lot of use of. For generated names that are entirely internal, I'm fine with making the names unique; however, Fields will be something that is public and important, and I'd rather it read more cleanly.
Maybe we should just make "Fields" a compiler reserved word?
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-623) Use a Java enum to represent field ids
in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-623:
---------------------------------
Patch Info: [Patch Available]
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12775261#action_12775261 ]
Todd Lipcon commented on THRIFT-623:
------------------------------------
bq. Maybe we should just make "Fields" a compiler reserved word?
I think Fields is a bit too likely to already be in use... I'm sort of tempted to think that we should namespace all thrift-internal stuff into a THRIFT namespace or something - MyType.THRIFT.Fields for example?
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12775262#action_12775262 ]
David Reiss commented on THRIFT-623:
------------------------------------
I'd prefer adding a single character than starting down the road of making tons of common words compiler-reserved.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776721#action_12776721 ]
David Reiss commented on THRIFT-623:
------------------------------------
Less likely to collide, but I think it would be better to not add a ton of poorly organized ad-hoc reserved words.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776714#action_12776714 ]
Nathan Marz commented on THRIFT-623:
------------------------------------
The generated Fields is an inner enum of the generated class... so it's fully qualified name will already be {full qualified generated classname}.Fields. This seems fine to me. However, I'd also be ok with adding an extra character - "_" is pretty ugly though. What if we call it "TFields"?
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12775052#action_12775052 ]
David Reiss commented on THRIFT-623:
------------------------------------
What would you think about renaming Fields to _Fields? I think it would be good to use leading underscores for all of our generated identifiers to reduce the risk of colliding with a user-specified name. As long as we are breaking source compatibility, it seems like a good time to make the change.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776736#action_12776736 ]
Bryan Duxbury commented on THRIFT-623:
--------------------------------------
I'm fairly certain we already assume that thrift field names don't start with underscores. However, your proposal was to put the underscore at the end.
Personally I just don't like putting an underscore on a name that actually is exposed to the user. Usually you only use that stuff internally.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776742#action_12776742 ]
David Reiss commented on THRIFT-623:
------------------------------------
bq. What would you think about renaming Fields to _Fields? I think it would be good to use leading underscores [...]
bq. However, your proposal was to put the underscore at the end.
Um...
bq. Personally I just don't like putting an underscore on a name that actually is exposed to the user. Usually you only use that stuff internally.
I agree, but in my mind the ugliness of ad-hoc reserved words trumps the ugliness of a leading underscore.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-623) Use a Java enum to represent field ids
in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-623:
---------------------------------
Attachment: thrift-623-v2.patch
Here's a cleaner version of the patch. This makes the changes a little more thorough.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-623) Use a Java enum to represent field ids
in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-623:
---------------------------------
Attachment: thrift-623-v3.patch
I've come around to the _Fields approach. This patch implements that. If no one has any objections, I will commit this later today.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623-v3.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-623) Use a Java enum to represent field
ids in generated structs
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776743#action_12776743 ]
Bryan Duxbury commented on THRIFT-623:
--------------------------------------
My bad. Don't know where I inverted that.
> Use a Java enum to represent field ids in generated structs
> -----------------------------------------------------------
>
> Key: THRIFT-623
> URL: https://issues.apache.org/jira/browse/THRIFT-623
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-623-v2.patch, thrift-623.patch
>
>
> The way things work today, field IDs get constants in their structs, but all the methods take ints as parameters when they actually want a field id. This is not the best, since it can lead to accidentally putting in field ids that don't exist, and we have to do extra work to make sure things behave correctly.
> Instead, we should make the field IDs into a proper Java enum class, and make the methods take that class as the parameter type. That way, we'll get the compiler's help enforcing our use of only legal field IDs, plus easier association with name info, docstrings, etc. All in all, it should amount to a usability improvement.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.