HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Return IEnumerable<KeyValuePair> from a private method; use Dictionary or anon. type?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
methodreturnprivateanontypekeyvaluepairdictionaryfromuseienumerable

Problem

Is it okay to return IEnumerable> from a private method instead of returning a read-only dictionary? This allows for shorter/simpler code.

Note: In the below code, PropertyDictionary is a static property that returns an IReadOnlyDictionary that was created through reflecting over the MyClass class looking for public, non-static, string-returning properties bearing a particular attribute. Also, the instance method that calls GetPropertyValues does some formatting, immediately changing the result into an IEnumerable, so the dictionary aspect isn't needed for long.

private static IEnumerable> GetPropertyValues(MyClass myClass) {
   return
      PropertyDictionary.ToDictionary(
         kvp => kvp.Key,
         kvp => (string) kvp.Value.GetValue(myClass, null)
      )
      .Where(kvp => !string.IsNullOrEmpty(kvp.Value));
}


vs.

private static IReadOnlyDictionary GetPropertyValues(MyClass myClass) {
   return
      new ReadOnlyDictionary(
         PropertyDictionary.ToDictionary(
            kvp => kvp.Key,
            kvp => (string) kvp.Value.GetValue(myClass, null)
         )
         .Where(kvp => !string.IsNullOrEmpty(kvp.Value))
         .ToDictionary(kvp => kvp.Key, kvp => kvp.Value)
      );
}


This converts to a dictionary twice. I wish there were a way to do Where and Select at the same time! Perhaps there is a better way. Can you suggest one?

Also, should I use an anonymous type instead of a dictionary for the intermediate step?

private static IEnumerable> GetPropertyValues(MyClass myClass) {
   return
      PropertyDictionary.Select(
         kvp => new {
            key = kvp.Key,
            value = (string) kvp.Value.GetValue(myClass, null)
         }
      )
      .Where(o => !string.IsNullOrEmpty(o.Value));
    // But now this isn't returning a Dictionary, but an anonymous object,
    // so this isn't going to work
}


and

```
private static IReadOnlyDictionary GetPropertyValues(MyClass myClass) {
return

Solution

I think the cleanest way to optimize it and to use only one loop is to first get the values with a select, create key/value pairs there and then filter by value:

private static IEnumerable> GetPropertyValues(MyClass myClass) {
   return
        PropertyDictionary
        .Select(kvp => 
            new KeyValuePair
            (
                kvp.Key,             
                (string)kvp.Value.GetValue(myClass, null)
            )
        )
        .Where(kvp => !string.IsNullOrEmpty(kvp.Value));
}

Code Snippets

private static IEnumerable<KeyValuePair<string, string>> GetPropertyValues(MyClass myClass) {
   return
        PropertyDictionary
        .Select(kvp => 
            new KeyValuePair<string, string>
            (
                kvp.Key,             
                (string)kvp.Value.GetValue(myClass, null)
            )
        )
        .Where(kvp => !string.IsNullOrEmpty(kvp.Value));
}

Context

StackExchange Code Review Q#98913, answer score: 4

Revisions (0)

No revisions yet.