Changes suggested during review.

- Remove the indexers in FieldAccessorTable
- Add a TODO for field ordering in oneof
pull/582/head
Jon Skeet 10 years ago
parent 6ea9bc7aa3
commit 0f34daad07
  1. 28
      csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs
  2. 3
      csharp/src/ProtocolBuffers/JsonFormatter.cs

@ -84,8 +84,8 @@ namespace Google.Protobuf.FieldAccess
public ReadOnlyCollection<OneofAccessor> Oneofs { get { return oneofs; } }
// TODO: Review the API for the indexers. Now that we have fields and oneofs, it's not as clear...
// TODO: Review this, as it's easy to get confused between FieldNumber and Index.
// Currently only used to get an accessor related to a oneof... maybe just make that simpler?
public IFieldAccessor this[int fieldNumber]
{
get
@ -94,29 +94,5 @@ namespace Google.Protobuf.FieldAccess
return accessors[field.Index];
}
}
public IFieldAccessor this[FieldDescriptor field]
{
get
{
if (field.ContainingType != descriptor)
{
throw new ArgumentException("FieldDescriptor does not match message type.");
}
return accessors[field.Index];
}
}
public OneofAccessor this[OneofDescriptor oneof]
{
get
{
if (oneof.ContainingType != descriptor)
{
throw new ArgumentException("OneofDescriptor does not match message type.");
}
return oneofs[oneof.Index];
}
}
}
}

@ -141,6 +141,7 @@ namespace Google.Protobuf
{
var descriptor = accessor.Descriptor;
// Oneofs are written later
// TODO: Change to write out fields in order, interleaving oneofs appropriately (as per binary format)
if (descriptor.ContainingOneof != null)
{
continue;
@ -176,7 +177,7 @@ namespace Google.Protobuf
{
continue;
}
var fieldAccessor = fields[fieldDescriptor];
var fieldAccessor = fields[fieldDescriptor.FieldNumber];
object value = fieldAccessor.GetValue(message);
// Omit awkward (single) values such as unknown enum values
if (!fieldDescriptor.IsRepeated && !fieldDescriptor.IsMap && !CanWriteSingleValue(fieldDescriptor, value))

Loading…
Cancel
Save