Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with required enum field with default value #103

Open
TeunVR opened this issue Jul 22, 2015 · 3 comments
Open

Problem with required enum field with default value #103

TeunVR opened this issue Jul 22, 2015 · 3 comments

Comments

@TeunVR
Copy link

TeunVR commented Jul 22, 2015

Consider the following .proto file:

message TestMessage {
    enum Result {
        OK = 0;
        FAILED = 1;
        TIMEOUT = 2;
    }
    required Result result = 1 [default = OK];
}

I would expect to be able to build a new TestMessage with:

TestMessage msg = TestMessage.CreateBuilder().Build();

This fails with:
An unhandled exception of type 'Google.ProtocolBuffers.UninitializedMessageException' occurred in Google.ProtocolBuffers.dll
Additional information: Message missing required fields: result

If i look at the generated code, the result-field is correctly set. However, the hasResult boolean is not set to true.

  public const int ResultFieldNumber = 1;
  private bool hasResult;
  private global::TestMessage.Types.Result result_ = global::TestMessage.Types.Result.OK;
  public bool HasResult {
    get { return hasResult; }
  }
  public global::TestMessage.Types.Result Result {
    get { return result_; }
  }

  public override bool IsInitialized {
    get {
      if (!hasResult) return false;
      return true;
    }
  }

I would expect the result field to have a default value, so i think hasResult should be initialized to true if a default value is set.

Note: using version 2.4.1.555

Or should this issue be submitted in https://github.com/google/protobuf ?

@jskeet
Copy link
Owner

jskeet commented Jul 22, 2015

Or should this issue be submitted in https://github.com/google/protobuf ?

No, absolutely not - the code there is very different now (at least on the csharp-experimental branch, which I hope to merge soon).

Your expectation here looks reasonable, but I don't expect to have the time to work on this any time soon (and this codebase is effectively legacy code at this point).

The simpler solution is just to make it an optional field - a required field with a default feels like a bit of a contradiction in terms. I realize this doesn't help if don't control the proto definition.

@TeunVR
Copy link
Author

TeunVR commented Jul 22, 2015

Thanks.
While i think a required field with a default value should work (for example: there must be a result-field and i don't have to set it if the result is OK) , i can workaround it by using an optional field as you suggested.

By the way: is the official csharp implementation in https://github.com/google/protobuf only going to be proto3?

@jskeet
Copy link
Owner

jskeet commented Jul 22, 2015

By the way: is the official csharp implementation in https://github.com/google/protobuf only going to be proto3?

That's my current plan, yes. It allows us to remove an awful lot of complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants