eerhardt commented on code in PR #34125:
URL: https://github.com/apache/arrow/pull/34125#discussion_r1117341439
##########
csharp/src/Apache.Arrow/Schema.cs:
##########
@@ -44,80 +43,74 @@ public partial class Schema
IEnumerable<Field> fields,
IEnumerable<KeyValuePair<string, string>> metadata)
{
- if (fields == null)
+ if (fields is null)
{
throw new ArgumentNullException(nameof(fields));
}
- _fields = fields.ToList();
-
- _fieldsDictionary = fields.ToDictionary(field => field.Name, field
=> field);
+ _fieldsList = fields.ToList();
+ FieldsLookup = _fieldsList.ToLookup(f => f.Name);
+ _fieldsDictionary = FieldsLookup.ToDictionary(g => g.Key, g =>
g.First());
Metadata = metadata?.ToDictionary(kv => kv.Key, kv => kv.Value);
}
- internal Schema(List<Field> fields, IReadOnlyDictionary<string,
string> metadata, bool copyCollections)
+ internal Schema(List<Field> fieldsList, IReadOnlyDictionary<string,
string> metadata, bool copyCollections)
{
- Debug.Assert(fields != null);
+ Debug.Assert(fieldsList != null);
Debug.Assert(copyCollections == false, "This internal constructor
is to not copy the collections.");
- _fields = fields;
-
- _fieldsDictionary = fields.ToDictionary(field => field.Name, field
=> field);
+ _fieldsList = fieldsList;
+ FieldsLookup = _fieldsList.ToLookup(f => f.Name);
+ _fieldsDictionary = FieldsLookup.ToDictionary(g => g.Key, g =>
g.First());
Metadata = metadata;
}
- public Field GetFieldByIndex(int i)
- {
- return _fields[i];
- }
+ public Field GetFieldByIndex(int i) => _fieldsList[i];
- public Field GetFieldByName(string name) =>
- Fields.TryGetValue(name, out Field field) ? field : null;
+ public Field GetFieldByName(string name) => FieldsLookup[name].First();
Review Comment:
```suggestion
public Field GetFieldByName(string name) =>
FieldsLookup[name].FirstOrDefault();
```
##########
csharp/src/Apache.Arrow/Ipc/ArrowStreamWriter.cs:
##########
@@ -522,13 +522,12 @@ private protected async Task WriteDictionaryAsync(Field
field, CancellationToken
long id = DictionaryMemo.GetId(field);
IArrowArray dictionary = DictionaryMemo.GetDictionary(id);
- var fieldsDictionary = new Dictionary<string, Field> {
- { dictionaryField.Name, dictionaryField } };
+ var fieldsList = new List<Field> { dictionaryField };
Review Comment:
```suggestion
var fields = new Field[] { dictionaryField };
```
Array implements `IReadOnlyList`, so we can just create an array here
instead of a List and save on an allocation.
##########
csharp/src/Apache.Arrow/Schema.cs:
##########
@@ -44,80 +43,74 @@ public partial class Schema
IEnumerable<Field> fields,
IEnumerable<KeyValuePair<string, string>> metadata)
{
- if (fields == null)
+ if (fields is null)
{
throw new ArgumentNullException(nameof(fields));
}
- _fields = fields.ToList();
-
- _fieldsDictionary = fields.ToDictionary(field => field.Name, field
=> field);
+ _fieldsList = fields.ToList();
+ FieldsLookup = _fieldsList.ToLookup(f => f.Name);
+ _fieldsDictionary = FieldsLookup.ToDictionary(g => g.Key, g =>
g.First());
Metadata = metadata?.ToDictionary(kv => kv.Key, kv => kv.Value);
}
- internal Schema(List<Field> fields, IReadOnlyDictionary<string,
string> metadata, bool copyCollections)
+ internal Schema(List<Field> fieldsList, IReadOnlyDictionary<string,
string> metadata, bool copyCollections)
{
- Debug.Assert(fields != null);
+ Debug.Assert(fieldsList != null);
Debug.Assert(copyCollections == false, "This internal constructor
is to not copy the collections.");
- _fields = fields;
-
- _fieldsDictionary = fields.ToDictionary(field => field.Name, field
=> field);
+ _fieldsList = fieldsList;
+ FieldsLookup = _fieldsList.ToLookup(f => f.Name);
+ _fieldsDictionary = FieldsLookup.ToDictionary(g => g.Key, g =>
g.First());
Metadata = metadata;
}
- public Field GetFieldByIndex(int i)
- {
- return _fields[i];
- }
+ public Field GetFieldByIndex(int i) => _fieldsList[i];
- public Field GetFieldByName(string name) =>
- Fields.TryGetValue(name, out Field field) ? field : null;
+ public Field GetFieldByName(string name) => FieldsLookup[name].First();
Review Comment:
Can you add a test here to ensure `null` is returned when a field doesn't
exist?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
