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.

Sign up for GitHub

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

Add TryGetExtension API (with TryGetOption) #7282

Closed
wants to merge 1 commit into from
Closed

Add TryGetExtension API (with TryGetOption) #7282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions csharp/src/Google.Protobuf.Test.TestProtos/TestMessagesProto2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3901,6 +3901,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestAllTypesProto2, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestAllTypesProto2,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestAllTypesProto2, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -4435,6 +4438,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<MessageSetCorrect, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<MessageSetCorrect,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<MessageSetCorrect, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down
32 changes: 31 additions & 1 deletion csharp/src/Google.Protobuf.Test.TestProtos/Unittest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4314,7 +4314,7 @@ public void MergeFrom(pb::CodedInputStream input) {
}

/// <summary>
/// This proto includes a recusively nested message.
/// This proto includes a recursively nested message.
/// </summary>
public sealed partial class NestedTestAllTypes : pb::IMessage<NestedTestAllTypes> {
private static readonly pb::MessageParser<NestedTestAllTypes> _parser = new pb::MessageParser<NestedTestAllTypes>(() => new NestedTestAllTypes());
Expand Down Expand Up @@ -5255,6 +5255,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestAllExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestAllExtensions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestAllExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -6030,6 +6033,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestGroupExtension, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestGroupExtension,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestGroupExtension, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -8601,6 +8607,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestEmptyMessageWithExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestEmptyMessageWithExtensions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestEmptyMessageWithExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -8742,6 +8751,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestMultipleExtensionRanges, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestMultipleExtensionRanges,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestMultipleExtensionRanges, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -12220,6 +12232,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestFieldOrderings, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestFieldOrderings,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestFieldOrderings, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -19366,6 +19381,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestPackedExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestPackedExtensions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestPackedExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -19507,6 +19525,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestUnpackedExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestUnpackedExtensions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestUnpackedExtensions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -20583,6 +20604,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestParsingMerge, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestParsingMerge,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestParsingMerge, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -23248,6 +23272,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestHugeFieldNumbers, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestHugeFieldNumbers,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestHugeFieldNumbers, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -23913,6 +23940,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<TestExtensionInsideTable, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<TestExtensionInsideTable,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<TestExtensionInsideTable, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down
21 changes: 21 additions & 0 deletions csharp/src/Google.Protobuf.Test/GeneratedMessageTest.Proto2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,27 @@ public void DefaultExtensionValues()
Assert.False(message.HasExtension(DefaultStringPieceExtension));
Assert.False(message.HasExtension(DefaultUint32Extension));
Assert.False(message.HasExtension(DefaultUint64Extension));

Assert.False(message.TryGetExtension(DefaultBoolExtension, out _));
Assert.False(message.TryGetExtension(DefaultBytesExtension, out _));
Assert.False(message.TryGetExtension(DefaultCordExtension, out _));
Assert.False(message.TryGetExtension(DefaultDoubleExtension, out _));
Assert.False(message.TryGetExtension(DefaultFixed32Extension, out _));
Assert.False(message.TryGetExtension(DefaultFixed64Extension, out _));
Assert.False(message.TryGetExtension(DefaultFloatExtension, out _));
Assert.False(message.TryGetExtension(DefaultForeignEnumExtension, out _));
Assert.False(message.TryGetExtension(DefaultImportEnumExtension, out _));
Assert.False(message.TryGetExtension(DefaultInt32Extension, out _));
Assert.False(message.TryGetExtension(DefaultInt64Extension, out _));
Assert.False(message.TryGetExtension(DefaultNestedEnumExtension, out _));
Assert.False(message.TryGetExtension(DefaultSfixed32Extension, out _));
Assert.False(message.TryGetExtension(DefaultSfixed64Extension, out _));
Assert.False(message.TryGetExtension(DefaultSint32Extension, out _));
Assert.False(message.TryGetExtension(DefaultSint64Extension, out _));
Assert.False(message.TryGetExtension(DefaultStringExtension, out _));
Assert.False(message.TryGetExtension(DefaultStringPieceExtension, out _));
Assert.False(message.TryGetExtension(DefaultUint32Extension, out _));
Assert.False(message.TryGetExtension(DefaultUint64Extension, out _));
}

[Test]
Expand Down
Binary file modified csharp/src/Google.Protobuf.Test/testprotos.pb
View file
Open in desktop
Binary file not shown.
16 changes: 16 additions & 0 deletions csharp/src/Google.Protobuf/ExtensionSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,22 @@ public static TValue Get<TTarget, TValue>(ref ExtensionSet<TTarget> set, Extensi
}
}

/// Gets the value of the specified extension if it's set.
public static bool TryGet<TTarget, TValue>(ref ExtensionSet<TTarget> set, Extension<TTarget, TValue> extension, out TValue value) where TTarget : IExtendableMessage<TTarget>
{
IExtensionValue extensionValue;
if (TryGetValue(ref set, extension, out extensionValue))
{
value = ((ExtensionValue<TValue>)extensionValue).GetValue();
return true;
}
else
{
value = default(TValue);
return false;
}
}

/// <summary>
/// Gets the value of the specified repeated extension or null if it doesn't exist in this set
/// </summary>
Expand Down
6 changes: 6 additions & 0 deletions csharp/src/Google.Protobuf/IExtendableMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ public interface IExtendableMessage<T> : IMessage<T> where T : IExtendableMessag
/// </summary>
TValue GetExtension<TValue>(Extension<T, TValue> extension);

/// <summary>
/// Gets the value of the specified extension.
/// Unlike <see cref="IExtendableMessage{T}.GetExtension{TValue}(Extension{T, TValue})"/>, this doesn't return the default value for the extension if it's not set.
/// </summary>
bool TryGetExtension<TValue>(Extension<T, TValue> extension, out TValue value);
Copy link
Contributor

@jtattermusch jtattermusch Apr 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the GetExtension method was a mistake from the very beginning, since it doesn't allow simple handling of the case where the extension is not present (which is a very common case in practice).

The problem seems to be that ExtensionSet.Get returns default(T) if the value is not found, which makes it impossible to distinguish the case where extension is not present at all vs where extension is present, but is set to the default value. (e.g for int, bool, float, etc..)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem seems to be that ExtensionSet.Get returns default(T) if the value is not found,

It returns the default value of the field which I believe matches up with get methods in other implementations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide and example how this is handled e.g. in java?
How can I distinguish between the extension value not being present at all from extension value being present, but set to its default value? Unless I'm missing something, it really feels that the API should allow you to do that. (does e.g. java allow that?).

Copy link
Contributor Author

@ObsidianMinor ObsidianMinor Apr 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

// in file scope
optional int32 bar = 99999 [default = 3];
// default
int value = msg.getExtension(FooFile.Bar); // 3

msg.setExtension(FooFile.Bar, 3);
if (msg.hasExtension(FooFile.Bar)) { // true
    value = msg.getExtension(FooFile.Bar); // 3
}

Custom options don't work the same way for us since java has builders and readers so they can just return the descriptor options reader and get the same API. We could add a HasOption API, but TryGet fixes the need for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Ok so to me it looks like we basically have two ways of handling this:

  • introduce TryGetExtension method, at which point GetOption becomes pretty much useless (because it can return default value for an extension that isn't present, which seems semantically wrong), but the issue is we already have introduced GetExtension, so getting rid of it shortly after introducing is odd.

  • we can keep GetExtension (because it already exists) and introduce a new HasExtension method which allows checking whether the value is present or not. If GetExtension() is called for a value that is not present, we can throw / return the default value (what the is getExtension method's behavior in java?).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getExtension returns the default value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I didn't notice that we already have a "HasExtension" method so my last comment is partially wrong.
Because we already have the HasExtension/GetExtension pair, I don't think we need TryGetExtension at all (why increase the API surface if all the necessary functionality is already there?).

In terms of options, I think both GetOption and TryGetOption are unnecessary and we should get rid of them. See my PR: #7434


/// <summary>
/// Gets the value of the specified repeated extension or null if the extension isn't registered in this set.
/// For a version of this method that never returns null, use <see cref="IExtendableMessage{T}.GetOrInitializeExtension{TValue}(RepeatedExtension{T, TValue})"/>
Expand Down
27 changes: 27 additions & 0 deletions csharp/src/Google.Protobuf/Reflection/Descriptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1730,6 +1730,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<ExtensionRangeOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<ExtensionRangeOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<ExtensionRangeOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -4868,6 +4871,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<FileOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<FileOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<FileOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -5277,6 +5283,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<MessageOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<MessageOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<MessageOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -5753,6 +5762,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<FieldOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<FieldOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<FieldOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -5948,6 +5960,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<OneofOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<OneofOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<OneofOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -6206,6 +6221,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<EnumOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<EnumOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<EnumOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -6419,6 +6437,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<EnumValueOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<EnumValueOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<EnumValueOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -6632,6 +6653,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<ServiceOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<ServiceOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<ServiceOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down Expand Up @@ -6886,6 +6910,9 @@ public void MergeFrom(pb::CodedInputStream input) {
public TValue GetExtension<TValue>(pb::Extension<MethodOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
public bool TryGetExtension<TValue>(pb::Extension<MethodOptions,TValue> extension, out TValue value) {
return pb::ExtensionSet.TryGet(ref _extensions, extension, out value);
}
public pbc::RepeatedField<TValue> GetExtension<TValue>(pb::RepeatedExtension<MethodOptions, TValue> extension) {
return pb::ExtensionSet.Get(ref _extensions, extension);
}
Expand Down
18 changes: 18 additions & 0 deletions csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,24 @@ public T GetOption<T>(Extension<EnumOptions, T> extension)
return value is IDeepCloneable<T> ? (value as IDeepCloneable<T>).Clone() : value;
}

/// <summary>
/// Gets a single value enum option for this descriptor
/// </summary>
public bool TryGetOption<T>(Extension<EnumOptions, T> extension, out T value)
{
T temp;
if (Proto.HasOptions && Proto.Options.TryGetExtension(extension, out temp))
{
value = temp is IDeepCloneable<T> ? (temp as IDeepCloneable<T>).Clone() : temp;
return true;
}
else
{
value = default(T);
return false;
}
}

/// <summary>
/// Gets a repeated value enum option for this descriptor
/// </summary>
Expand Down
18 changes: 18 additions & 0 deletions csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,24 @@ public T GetOption<T>(Extension<EnumValueOptions, T> extension)
return value is IDeepCloneable<T> ? (value as IDeepCloneable<T>).Clone() : value;
}

/// <summary>
/// Gets a single value enum value option for this descriptor
/// </summary>
public bool TryGetOption<T>(Extension<EnumValueOptions, T> extension, out T value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this method is basically just duplicating the Extension API and we should avoid that.
See #7434 and let me know what you think.

{
T temp;
if (Proto.HasOptions && Proto.Options.TryGetExtension(extension, out temp))
{
value = temp is IDeepCloneable<T> ? (temp as IDeepCloneable<T>).Clone() : temp;
return true;
}
else
{
value = default(T);
return false;
}
}

/// <summary>
/// Gets a repeated value enum value option for this descriptor
/// </summary>
Expand Down
18 changes: 18 additions & 0 deletions csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,24 @@ public T GetOption<T>(Extension<FieldOptions, T> extension)
return value is IDeepCloneable<T> ? (value as IDeepCloneable<T>).Clone() : value;
}

/// <summary>
/// Gets a single value field option for this descriptor
/// </summary>
public bool TryGetOption<T>(Extension<FieldOptions, T> extension, out T value)
{
T temp;
if (Proto.HasOptions && Proto.Options.TryGetExtension(extension, out temp))
{
value = temp is IDeepCloneable<T> ? (temp as IDeepCloneable<T>).Clone() : temp;
return true;
}
else
{
value = default(T);
return false;
}
}

/// <summary>
/// Gets a repeated value field option for this descriptor
/// </summary>
Expand Down
Loading