You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "William Blinn (JIRA)" <ji...@apache.org> on 2011/03/31 23:03:09 UTC

[jira] [Created] (THRIFT-1127) C# should not generate default constructor

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


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.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (THRIFT-1127) C# should not generate default constructor

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

Vadim Chekan commented on THRIFT-1127:
--------------------------------------

Why not init fields with default values instead of doing it in default constructor?
If init fields with default values, it allows dropping default constructor unconditionally. Which allows more flexibility in partial classes, for example override default value conditionaly:
"partial class C { C() {if(Something) _x = NOT_DEFAULT;} }

> 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.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (THRIFT-1127) C# should not generate default constructor

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

Thunder Stumpges commented on THRIFT-1127:
------------------------------------------

The point of the default constructor is to initialize member default values. However in a case where you have none (or need to have none in order to provide your own), I agree that it should not generate one. I am working on a patch that will generate the default constructor (as it does today) only if there are one or more members that have default values specified. I believe this should work for you (it works for me!)

> 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
>
> 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.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (THRIFT-1127) C# should not generate default constructor

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

Thunder Stumpges commented on THRIFT-1127:
------------------------------------------

OK, well I got my environment to build, and the attached patch works wonderfully for me! Hope to see this one get into a build soon!

> 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.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (THRIFT-1127) C# should not generate default constructor

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

Thunder Stumpges updated THRIFT-1127:
-------------------------------------

    Attachment: DontCreateEmptyDefaultConstructor.patch

Here is a patch which will only generate a default constructor if there are some field values to set. This SHOULD work however I have not compiled it since I do not yet have a working build environment. Any chance someone could review it and build it for me? Really need this one fixed!

> 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.
For more information on JIRA, see: http://www.atlassian.com/software/jira