Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Helper to enumerate IConfiguration? #370

Closed
davidfowl opened this issue Jan 17, 2016 · 18 comments
Closed

Helper to enumerate IConfiguration? #370

davidfowl opened this issue Jan 17, 2016 · 18 comments
Assignees
Milestone

Comments

@davidfowl
Copy link
Member

I was trying to copy an IConfiguration into a flat dicitonary and came up with this (completely untested) method:

var stack = new Stack<IConfiguration>();
stack.Push(configuration);

while (stack.Count > 0)
{
    var config = stack.Pop();
    var section = config as IConfigurationSection;

    if (section != null)
    {
        yield return new KeyValuePair<string, string>(section.Path, section.Value);
    }

    foreach (var child in config.GetChildren())
    {
        stack.Push(child);
    }
}

It would be useful to have something that expanded all of the kvps without doing manual recursion.

/cc @divega @HaoK

@divega
Copy link

divega commented Jan 19, 2016

Looks good!

I was trying to copy an IConfiguration into a flat dicitonary

Curious, do you see this as something many customers will need to do or as a rather advanced scenario?

FWIW, we could have had IConfiguration: IEnumerable<KeyValuePair<string, string>> directly but decided against it to avoid some unwanted side effects (like having the full set of LINQ operators show up in Intellisense). We could have an extension method for this instead.

@davidfowl
Copy link
Member Author

Yes, see aspnet/Hosting#582 where I ran into this.

@divega divega assigned HaoK and unassigned divega Jan 29, 2016
@divega divega added this to the 1.0.0-rc2 milestone Jan 29, 2016
@JunTaoLuo
Copy link
Contributor

I added the suggested enumeration as an extension method as part of aspnet/Hosting#596. Once it is implemented in Configuration, I'll remove the redundant extension in Hosting.

@divega
Copy link

divega commented Jan 29, 2016

@JunTaoLuo Yes I was looking at it this second. Thanks for the heads up.

@HaoK Let's have this as a public extension method in Configuration (it could live side-byside with the GetValue<T>() stuff). Need to decide about naming:

  • 'ToKeyValuePairs()`
  • Flatten()
  • GetFlattenedSettings()

etc.

@HaoK
Copy link
Member

HaoK commented Feb 23, 2016

I like IEnumerable<KeyValuePair<string, string>> Flatten(this IConfiguration configuration)

@HaoK
Copy link
Member

HaoK commented Feb 23, 2016

@divega What GetValue stuff? The ones that live in the binder? or did you want me to move those to Configuration now as part of adding Flatten?

@divega
Copy link

divega commented Feb 24, 2016

I was referring to having this as an extension method in configuration alongside GetValue(), but this is a completely independent change.

I am not sure about the naming. Of the three options I wrote down I think Flatten is the one that is going to require looking into documentation the most in order to understand what it does, but I guess once you know what it does, it is the shortest 😄 Given that it is most likely not going to be a very commonly used API I would optimize for clarity.

@HaoK
Copy link
Member

HaoK commented Feb 24, 2016

Well GetFlattenedSettings doesn't really make sense given the current config apis which don't use the term settings anywhere. I don't really think ToKeyValuePairs actually is very clear compared to something with the word Flatten in it.

Since this is just a DepthFirstTraversal, maybe that makes it clear?

DepthFirstEnumeration()
DepthFirstTraversal()

@HaoK
Copy link
Member

HaoK commented Feb 24, 2016

Actually why can't we just call this Enumerate?

@divega
Copy link

divega commented Feb 25, 2016

I am not completely against any of these names besides probably the "depth first" ones, as I don't think the fact that is depth first is going to play much of a role 😄 Thinking of Enumerate() I also believe the existing LINQ idiom AsEnumerable() could work. The method would return an IEnumerable of KeyValuePair<string,string> which is a bit complicated but conceptually key value pairs is what the configuration is made of, so I don't think it would be going too far.

cc @shirhatti @danroth27

@MichaCo
Copy link
Contributor

MichaCo commented Feb 25, 2016

Well known terms (from linq) are ToList, ToArray, ToDictionary or AsEnumerable.
I'd use
ToDictionary returning IDictionary<string, string>), or
AsEnumerable returning IEnumerable<KeyValuePair<string, string>>

@HaoK
Copy link
Member

HaoK commented Feb 25, 2016

I like AsEnumerable as well

@HaoK
Copy link
Member

HaoK commented Feb 25, 2016

We could do both of those actually ToDictionary/AsEnumerable

@MichaCo
Copy link
Contributor

MichaCo commented Feb 25, 2016

👍

@divega
Copy link

divega commented Feb 25, 2016

Having AsEnumerable() is nice because unlike ToDictionary() it streams. Moreover once you have AsEnumerable() getting a dictionary is pretty easy using the existing LINQ operators:

var dictionary = configuration.AsEnumerable().ToDictionary(p => p.Key, x => p.Value);

So I don't think we would need to add ToDictionary().

@HaoK
Copy link
Member

HaoK commented Feb 25, 2016 via email

@divega
Copy link

divega commented Feb 25, 2016

Mostly because it would automatically pollute our nice API with several dozen LINQ operators 😄 See my previous comment at #370 (comment).

@HaoK
Copy link
Member

HaoK commented Feb 26, 2016

b958a68

@HaoK HaoK closed this as completed Feb 26, 2016
@HaoK HaoK added 3 - Done and removed 2 - Working labels Feb 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants