Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4715

C# union "data" should be strongly-typed

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 0.12.0
    • Fix Version/s: 0.13.0
    • Component/s: C# - Compiler
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      `union` option for netcore generator was fixed in 0.12, but what it generates doesn't seem very user-friendly.

      Following thrift:
      ```thrift
      struct PlayMsg

      { 1: string url, }

      union RequestMsg

      { 1: PlayMsg Play, }

      ```

      Generates:

      public abstract partial class RequestMsg : TAbstractBase {
      public abstract void Write(TProtocol protocol);
      public readonly bool Isset;
      public abstract object Data { get; }
      protected RequestMsg(bool isset) {
      Isset = isset;
      }
      
      public class ___undefined : RequestMsg {
      public override object Data { get { return null; } }
      public ___undefined() : base(false) {}
      
      public override void Write(TProtocol protocol) {
      throw new TProtocolException( TProtocolException.INVALID_DATA, "Cannot persist an union type which is not set.");
      }
      
      }
      
      public class Play : RequestMsg {
      private PlayMsg _data;
      public override object Data { get { return _data; } }
      public Play(PlayMsg data) : base(true) {
      this._data = data;
      }
      //SNIP

      Usage:

      // RequestMsg message = ...
      switch (message)
      {
      case RequestMsg.Play msg:
          // Need a cast here T_T
          PlayMsg play = (PlayMsg)msg.Data;
      

      Is there a reason for the `public abstract object Data`?

      If we get rid of that and instead generate a strongly-type getter we don't need to cast `Data`:

      public class Play : RequestMsg{ 
          public PlayMsg Data { get; private set; }
          public Play(PlayMsg data) : base(true)
          { this.Data = data; }
      //SNIP
      

       

       

      I could have sworn it worked like that with the "csharp" generator in 0.11.0 but it generates the same now.

      Is the intended usage different from what I'm doing?

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jensg Jens Geyer
                Reporter:
                jeikabu J W
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 20m
                  20m