From 0f34daad07153a66b492ab938e85f17e82b91706 Mon Sep 17 00:00:00 2001
From: Jon Skeet <jonskeet@google.com>
Date: Tue, 14 Jul 2015 09:41:28 +0100
Subject: Changes suggested during review.

- Remove the indexers in FieldAccessorTable
- Add a TODO for field ordering in oneof
---
 .../FieldAccess/FieldAccessorTable.cs              | 28 ++--------------------
 csharp/src/ProtocolBuffers/JsonFormatter.cs        |  3 ++-
 2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs
index c69612fb..80be93f5 100644
--- a/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.cs
+++ b/csharp/src/ProtocolBuffers/FieldAccess/FieldAccessorTable.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];
-            }
-        }
     }
 }
\ No newline at end of file
diff --git a/csharp/src/ProtocolBuffers/JsonFormatter.cs b/csharp/src/ProtocolBuffers/JsonFormatter.cs
index 3e2244d6..9bd628eb 100644
--- a/csharp/src/ProtocolBuffers/JsonFormatter.cs
+++ b/csharp/src/ProtocolBuffers/JsonFormatter.cs
@@ -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))
-- 
cgit v1.2.3