You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Kevin Clark (JIRA)" <ji...@apache.org> on 2009/03/24 16:47:58 UTC

[jira] Closed: (THRIFT-161) Generated code doesn't honor "optional" flag when marshaling struct, and uses invalid "HashSet" class for set

     [ https://issues.apache.org/jira/browse/THRIFT-161?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kevin Clark closed THRIFT-161.
------------------------------

       Resolution: Fixed
    Fix Version/s: 0.1

Ok, I'm tentatively closing this. If it's still an issue (uncovered by those other tickets) feel free to reopen.

> Generated code doesn't honor "optional" flag when marshaling struct, and uses invalid "HashSet" class for set<foo>
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-161
>                 URL: https://issues.apache.org/jira/browse/THRIFT-161
>             Project: Thrift
>          Issue Type: Bug
>          Components: Compiler (C#)
>            Reporter: Dave Engberg
>             Fix For: 0.1
>
>         Attachments: t_csharp_generator.cc.patch
>
>
> The C# generated code emits values for every primitive field, even if that field was declared as "optional" in the IDL and never set.  This is unlike the Java generated code which checks the corresponding __isset first.
> Also, if the IDL declares a value of type set<foo>, the generator tries to create a corresponding C# structure based on "HashSet", which is not a valid C# type.  (I think this was probably a cut-and-paste error from the Java generator.)
> My patch addresses the first problem by checking __isset before writing a value for an optional primitive field.  To make this easier to manage, this also generates a C# "property" for each field on the struct, which automatically sets the __isset field when the property is set.  E.g.:
>     public string title;
>     public string Title {
>       get { return title; }
>       set { title = value; __isset.title = true; }
>     }
> In clean, well-encapsulated C# code, the corresponding member (in this case, "title") would be declared "private" so that all operations on the field would go through the accessor property (e.g. "Title").  Unfortunately, this would break backward compatibility for existing C# Thrift consumers, so I didn't make this change.  However, I'd recommend that new consumers use the property to set the field rather than the raw member variable.
> For the HashSet<> problem, I just replaced the occurrencies of HashSet with the C# List<> class.  This will compile and work correctly, but it will not give O(logn) performance for set-based operations as consumers may expect (and it will also not automatically remove duplicates).  Unfortunately, it does not appear that the standard C# collections framework includes a Set<> class that can be used, so I don't see a great alternative without bundling our own Set implementation.
> Note that this patch includes the changes I submitted in THRIFT-159:  https://issues.apache.org/jira/browse/THRIFT-159
> If necessary, I can pull these apart into separate patches, however.

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