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

Jens Geyer commented on THRIFT-1127:
------------------------------------

Hi Tunder,

{quote}
1. Use of auto-properties for required members with default values:
I had to change the logic in deciding between private field backed property and 
auto-property for Required members. If we use auto-property, we can't do the 
field level initialization. Any time we have a default, we need a field backed 
property.
{quote}

I know, and it's fine with me. 

{quote}
This undoes a little bit of the work done in bug THRIFT-1783. What I did was 
add the field-backed property if a default value was present, but leave out the 
stuff for _isset when it is required. I am somewhat uncomfortable doing this, 
though I don't fully understand why the fixing of bug 1783 required removal of 
the field-backed properties, or the _isset either. Was it just an optimization? 
Does this sound OK?
{quote}

Optimization, intended as an attempt to keep the code concise wherever 
possible. Note that in the cases where a backing field is needed (e.g. 
optionals), it is still generated. So this is fine with me, I will not lose 
much sleep over that.

{quote}
2. Required Member Constructor (from bug THRIFT-1783):
Bug THRIFT-1783 creates a constructor taking all required members. [...] I 
would like to get rid of this constructor. It's easy enough to just use object 
initialization syntax. Is this OK?
{quote}

For me personally it would be ok, but I don't know if there might be any 
requirements that I'm not aware of. Maybe Carl can shed some light on this.

{quote}
3. Set initialization with const value: 
For some reason (not sure why, do you?) The type name rendered for Set is not 
just the .net HashSet<T> but it uses a thrift version called THashSet<T> which 
does not have a constructor taking IEnumerable<T> like the List<T>, 
Dictionary<K,V>, and HashSet<T> .net versions do. 
{quote}

The repo says, that THashSet.cs has been introduced with THRIFT-309 for Mono 
compatibility. I don't know much about that issue. But how about just adding 
the missing CTOR in THashSet?

Jens
                
> C# should not generate default constructor
> ------------------------------------------
>
>                 Key: THRIFT-1127
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1127
>             Project: Thrift
>          Issue Type: Bug
>          Components: C# - Compiler
>    Affects Versions: 0.5
>            Reporter: William Blinn
>         Attachments: DontCreateEmptyDefaultConstructor.patch
>
>
> The C# code generator should not produce a default constructor. 
> Thrift generates partial classes for thrift structs, meaning that the class 
> may be spread across multiple files and csc will link them to be a separate 
> file. When the thrift generated class has the partial constructor, it cannot 
> be added in other files. This is a problem if you want to implement a default 
> constructor that does some initialization to the data in the class.
> For example, this thrift code:
> {code}
> struct DateTime
> {
>     1: required i64 ticks,
> }
> {code}
> produces
> {code}
> /**
>  * Autogenerated by Thrift
>  *
>  * DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING
>  */
> using System;
> using System.Collections;
> using System.Collections.Generic;
> using System.Text;
> using System.IO;
> using Thrift;
> using Thrift.Collections;
> using Thrift.Protocol;
> using Thrift.Transport;
> namespace Thrift.Generated
> {
>   [Serializable]
>   public partial class DateTime : TBase
>   {
>     private long _ticks;
>     public long Ticks
>     {
>       get
>       {
>         return _ticks;
>       }
>       set
>       {
>         __isset.ticks = true;
>         this._ticks = value;
>       }
>     }
>     public Isset __isset;
>     [Serializable]
>     public struct Isset {
>       public bool ticks;
>     }
>     public DateTime() {
>     }
>     public void Read (TProtocol iprot)
> ...
> {code}
> It would be great if it instead produced code like this:
> {code}
> /**
>  * Autogenerated by Thrift
>  *
>  * DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING
>  */
> using System;
> using System.Collections;
> using System.Collections.Generic;
> using System.Text;
> using System.IO;
> using Thrift;
> using Thrift.Collections;
> using Thrift.Protocol;
> using Thrift.Transport;
> namespace Thrift.Generated
> {
>   [Serializable]
>   public partial class DateTime : TBase
>   {
>     private long _ticks;
>     public long Ticks
>     {
>       get
>       {
>         return _ticks;
>       }
>       set
>       {
>         __isset.ticks = true;
>         this._ticks = value;
>       }
>     }
>     public Isset __isset;
>     [Serializable]
>     public struct Isset {
>       public bool ticks;
>     }
>     public void Read (TProtocol iprot)
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to