Recursive conversion from ExpandoObject to DictionaryDictionary of attributes and valuesCompare dicts and...
Minimizing medical costs with HSA
Do intermediate subdomains need to exist?
What does the ash content of broken wheat really mean?
Taking advantage when the HR forgets to communicate the rules
Why did the "Orks" never develop better firearms than Firelances and Handcannons?
Isn't "Dave's protocol" good if only the database, and not the code, is leaked?
How to supply water to a coastal desert town with no rain and no freshwater aquifers?
Do I need to be legally qualified to install a Hive smart thermostat?
Explain how 'Sharing the burden' puzzle from Professor Layton and the Miracle Mask should be solved
Recursive conversion from ExpandoObject to Dictionary<string, object>
How can one synthesise a conjugated alkyne chain?
Could you sell yourself into slavery in the USA?
Advice for making/keeping shredded chicken moist?
How can solar sailed ships be protected from space debris?
Does a multiclassed wizard start with a spellbook?
Short SF story , future people who feed on sunlight try solid food
Should I warn my boss I might take sick leave
Convenience stores in India
Can a wizard delay learning new spells from leveling up, and instead learn different spells later?
Speeding up thousands of string parses
How should I present a resort brochure in my general fiction?
Should I cheat if the majority does it?
how can i make this execution plan more efficient?
What instances can be solved today by modern solvers (pure LP)?
Recursive conversion from ExpandoObject to Dictionary
Dictionary of attributes and valuesCompare dicts and record changesMatch two dictionary keys efficiently in PythonGet all combination of a nested object with arbitrary levelsWriting an anagram efficientlyList all possible permutations from a python dictionary of listshow to filter text from a string and convert it to dictionary and check for matching values in pythonRecursive object/array filter by keyRecursive parser from Binary to JSON outputBuilding nested dictionaries to dump it to .json later
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty{ margin-bottom:0;
}
$begingroup$
For my blazor library which is a modification of this awesome library I have to convert an ExpandoObject
into a Dictionary<string, object>
since ExpandoObject
s aren't serialized properly in the newest preview versions of dotnet-core. See my question related to this for more details.
My current approach goes as follows:
Dictionary<string, object> ConvertDynamicToDictonary(IDictionary<string, object> value)
{
return value.ToDictionary(
p => p.Key,
p =>
{
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict);
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return p.Value;
}
);
}
This works fine but I'm not sure about the IEnumerable
part. If I omit the list.Any
check it still works fine. New version (only write the IEnumerable
part changed):
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
// take all ExpandoObjects and go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
I have also tried using IEnumerable<ExpandoObject>
instead of just IEnumerable<object>
to see if that would work and maybe be cleaner and I can confirm that that does not work (throws error).
What I would like to know now is, if it's a good idea to omit the Any
check and which option is more performant (and why) and/or cleaner.
Also please mention every small thing that bugs you, I always want to make it as perfect as possible :)
I first wanted to post this on StackOverflow but I think it fits better on here - correct me if I'm wrong.
EDIT 1:
Because I feel like I have not made clear how I will use this function, I have some demo-code for you. This will of course not compile, it's just to show how and why I need this method.
If this edit is problematic because I'm adding new code, I will post a new, more complete question later on (I sadly don't have anymore time today).
// this class (like all the others) are of course much more complicated than this.
// There's also a lot of abstraction etc (you can see the actual code on the github I've linked at the start).
SomeConfig config = new SomeConfig
{
Options = new SomeOptions
{
SomeInt = 2,
SomeString = null, // any trace of this property will not be present in the expando object
Axes = new List<Axis>
{
new Axis() // this axis will also be an expandoObject after being extracted from the json
}
},
Data = new SomeData
{
Data = new List<int> { 1, 2, 3, 4, 5 },
SomeString = "asdf"
}
};
dynamic expando = /* ParseConfigToJsonWithoutNulls -> ParseJsonToExpandoObject */
IJSRuntime jsRT = GetTheRuntime();
jsRT.InvokeAsync("blabla", expando); // this would NOT work because ExpandoObject cannot be serialized to json correctly and throws a runtime error
// this method is what we need in the best way possible without missing something :)
Dictionary<string, object> dictFromExpando = ConvertExpandoToDictonary(expando);
// so the only purpose of the ConvertExpandoToDictonary is to recursively convert all the ExpandoObjects to Dictionaries as they are serializable
// I have to do this recursively since there are nested ExpandoObjects that were created when parsing the huge json to the ExpandoObject with Json.Net
// I have decided to go with Dictionaries because it works and ExpandoObject implements the interface IDictionary<string, object>
jsRT.InvokeAsync("blabla", dictFromExpando); // this now works perfectly fine
```
c# recursion dictionary
New contributor
$endgroup$
add a comment |
$begingroup$
For my blazor library which is a modification of this awesome library I have to convert an ExpandoObject
into a Dictionary<string, object>
since ExpandoObject
s aren't serialized properly in the newest preview versions of dotnet-core. See my question related to this for more details.
My current approach goes as follows:
Dictionary<string, object> ConvertDynamicToDictonary(IDictionary<string, object> value)
{
return value.ToDictionary(
p => p.Key,
p =>
{
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict);
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return p.Value;
}
);
}
This works fine but I'm not sure about the IEnumerable
part. If I omit the list.Any
check it still works fine. New version (only write the IEnumerable
part changed):
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
// take all ExpandoObjects and go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
I have also tried using IEnumerable<ExpandoObject>
instead of just IEnumerable<object>
to see if that would work and maybe be cleaner and I can confirm that that does not work (throws error).
What I would like to know now is, if it's a good idea to omit the Any
check and which option is more performant (and why) and/or cleaner.
Also please mention every small thing that bugs you, I always want to make it as perfect as possible :)
I first wanted to post this on StackOverflow but I think it fits better on here - correct me if I'm wrong.
EDIT 1:
Because I feel like I have not made clear how I will use this function, I have some demo-code for you. This will of course not compile, it's just to show how and why I need this method.
If this edit is problematic because I'm adding new code, I will post a new, more complete question later on (I sadly don't have anymore time today).
// this class (like all the others) are of course much more complicated than this.
// There's also a lot of abstraction etc (you can see the actual code on the github I've linked at the start).
SomeConfig config = new SomeConfig
{
Options = new SomeOptions
{
SomeInt = 2,
SomeString = null, // any trace of this property will not be present in the expando object
Axes = new List<Axis>
{
new Axis() // this axis will also be an expandoObject after being extracted from the json
}
},
Data = new SomeData
{
Data = new List<int> { 1, 2, 3, 4, 5 },
SomeString = "asdf"
}
};
dynamic expando = /* ParseConfigToJsonWithoutNulls -> ParseJsonToExpandoObject */
IJSRuntime jsRT = GetTheRuntime();
jsRT.InvokeAsync("blabla", expando); // this would NOT work because ExpandoObject cannot be serialized to json correctly and throws a runtime error
// this method is what we need in the best way possible without missing something :)
Dictionary<string, object> dictFromExpando = ConvertExpandoToDictonary(expando);
// so the only purpose of the ConvertExpandoToDictonary is to recursively convert all the ExpandoObjects to Dictionaries as they are serializable
// I have to do this recursively since there are nested ExpandoObjects that were created when parsing the huge json to the ExpandoObject with Json.Net
// I have decided to go with Dictionaries because it works and ExpandoObject implements the interface IDictionary<string, object>
jsRT.InvokeAsync("blabla", dictFromExpando); // this now works perfectly fine
```
c# recursion dictionary
New contributor
$endgroup$
2
$begingroup$
Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 3 → 2
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
7 hours ago
$begingroup$
@SᴀᴍOnᴇᴌᴀ Sorry I did not know. I feel like this was just a small mistake I made which may have caused some misunderstandings. Do I really have to ask a new question just for that (becaues I feel like my main concern didn't have to do with that at all) or how would you recommend me clearing that misunderstanding up to the concerned question?
$endgroup$
– Joelius
7 hours ago
2
$begingroup$
I would await sufficient answers and comments. Then you could self-answer or accept if you please, and perhaps also ask a follow-up question.
$endgroup$
– dfhwze
7 hours ago
1
$begingroup$
The rollback is being discussed on meta.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
6 hours ago
add a comment |
$begingroup$
For my blazor library which is a modification of this awesome library I have to convert an ExpandoObject
into a Dictionary<string, object>
since ExpandoObject
s aren't serialized properly in the newest preview versions of dotnet-core. See my question related to this for more details.
My current approach goes as follows:
Dictionary<string, object> ConvertDynamicToDictonary(IDictionary<string, object> value)
{
return value.ToDictionary(
p => p.Key,
p =>
{
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict);
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return p.Value;
}
);
}
This works fine but I'm not sure about the IEnumerable
part. If I omit the list.Any
check it still works fine. New version (only write the IEnumerable
part changed):
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
// take all ExpandoObjects and go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
I have also tried using IEnumerable<ExpandoObject>
instead of just IEnumerable<object>
to see if that would work and maybe be cleaner and I can confirm that that does not work (throws error).
What I would like to know now is, if it's a good idea to omit the Any
check and which option is more performant (and why) and/or cleaner.
Also please mention every small thing that bugs you, I always want to make it as perfect as possible :)
I first wanted to post this on StackOverflow but I think it fits better on here - correct me if I'm wrong.
EDIT 1:
Because I feel like I have not made clear how I will use this function, I have some demo-code for you. This will of course not compile, it's just to show how and why I need this method.
If this edit is problematic because I'm adding new code, I will post a new, more complete question later on (I sadly don't have anymore time today).
// this class (like all the others) are of course much more complicated than this.
// There's also a lot of abstraction etc (you can see the actual code on the github I've linked at the start).
SomeConfig config = new SomeConfig
{
Options = new SomeOptions
{
SomeInt = 2,
SomeString = null, // any trace of this property will not be present in the expando object
Axes = new List<Axis>
{
new Axis() // this axis will also be an expandoObject after being extracted from the json
}
},
Data = new SomeData
{
Data = new List<int> { 1, 2, 3, 4, 5 },
SomeString = "asdf"
}
};
dynamic expando = /* ParseConfigToJsonWithoutNulls -> ParseJsonToExpandoObject */
IJSRuntime jsRT = GetTheRuntime();
jsRT.InvokeAsync("blabla", expando); // this would NOT work because ExpandoObject cannot be serialized to json correctly and throws a runtime error
// this method is what we need in the best way possible without missing something :)
Dictionary<string, object> dictFromExpando = ConvertExpandoToDictonary(expando);
// so the only purpose of the ConvertExpandoToDictonary is to recursively convert all the ExpandoObjects to Dictionaries as they are serializable
// I have to do this recursively since there are nested ExpandoObjects that were created when parsing the huge json to the ExpandoObject with Json.Net
// I have decided to go with Dictionaries because it works and ExpandoObject implements the interface IDictionary<string, object>
jsRT.InvokeAsync("blabla", dictFromExpando); // this now works perfectly fine
```
c# recursion dictionary
New contributor
$endgroup$
For my blazor library which is a modification of this awesome library I have to convert an ExpandoObject
into a Dictionary<string, object>
since ExpandoObject
s aren't serialized properly in the newest preview versions of dotnet-core. See my question related to this for more details.
My current approach goes as follows:
Dictionary<string, object> ConvertDynamicToDictonary(IDictionary<string, object> value)
{
return value.ToDictionary(
p => p.Key,
p =>
{
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict);
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return p.Value;
}
);
}
This works fine but I'm not sure about the IEnumerable
part. If I omit the list.Any
check it still works fine. New version (only write the IEnumerable
part changed):
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (p.Value is IEnumerable<object> list)
{
// take all ExpandoObjects and go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
I have also tried using IEnumerable<ExpandoObject>
instead of just IEnumerable<object>
to see if that would work and maybe be cleaner and I can confirm that that does not work (throws error).
What I would like to know now is, if it's a good idea to omit the Any
check and which option is more performant (and why) and/or cleaner.
Also please mention every small thing that bugs you, I always want to make it as perfect as possible :)
I first wanted to post this on StackOverflow but I think it fits better on here - correct me if I'm wrong.
EDIT 1:
Because I feel like I have not made clear how I will use this function, I have some demo-code for you. This will of course not compile, it's just to show how and why I need this method.
If this edit is problematic because I'm adding new code, I will post a new, more complete question later on (I sadly don't have anymore time today).
// this class (like all the others) are of course much more complicated than this.
// There's also a lot of abstraction etc (you can see the actual code on the github I've linked at the start).
SomeConfig config = new SomeConfig
{
Options = new SomeOptions
{
SomeInt = 2,
SomeString = null, // any trace of this property will not be present in the expando object
Axes = new List<Axis>
{
new Axis() // this axis will also be an expandoObject after being extracted from the json
}
},
Data = new SomeData
{
Data = new List<int> { 1, 2, 3, 4, 5 },
SomeString = "asdf"
}
};
dynamic expando = /* ParseConfigToJsonWithoutNulls -> ParseJsonToExpandoObject */
IJSRuntime jsRT = GetTheRuntime();
jsRT.InvokeAsync("blabla", expando); // this would NOT work because ExpandoObject cannot be serialized to json correctly and throws a runtime error
// this method is what we need in the best way possible without missing something :)
Dictionary<string, object> dictFromExpando = ConvertExpandoToDictonary(expando);
// so the only purpose of the ConvertExpandoToDictonary is to recursively convert all the ExpandoObjects to Dictionaries as they are serializable
// I have to do this recursively since there are nested ExpandoObjects that were created when parsing the huge json to the ExpandoObject with Json.Net
// I have decided to go with Dictionaries because it works and ExpandoObject implements the interface IDictionary<string, object>
jsRT.InvokeAsync("blabla", dictFromExpando); // this now works perfectly fine
```
c# recursion dictionary
c# recursion dictionary
New contributor
New contributor
edited 6 hours ago
Joelius
New contributor
asked 8 hours ago
JoeliusJoelius
1112 bronze badges
1112 bronze badges
New contributor
New contributor
2
$begingroup$
Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 3 → 2
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
7 hours ago
$begingroup$
@SᴀᴍOnᴇᴌᴀ Sorry I did not know. I feel like this was just a small mistake I made which may have caused some misunderstandings. Do I really have to ask a new question just for that (becaues I feel like my main concern didn't have to do with that at all) or how would you recommend me clearing that misunderstanding up to the concerned question?
$endgroup$
– Joelius
7 hours ago
2
$begingroup$
I would await sufficient answers and comments. Then you could self-answer or accept if you please, and perhaps also ask a follow-up question.
$endgroup$
– dfhwze
7 hours ago
1
$begingroup$
The rollback is being discussed on meta.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
6 hours ago
add a comment |
2
$begingroup$
Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 3 → 2
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
7 hours ago
$begingroup$
@SᴀᴍOnᴇᴌᴀ Sorry I did not know. I feel like this was just a small mistake I made which may have caused some misunderstandings. Do I really have to ask a new question just for that (becaues I feel like my main concern didn't have to do with that at all) or how would you recommend me clearing that misunderstanding up to the concerned question?
$endgroup$
– Joelius
7 hours ago
2
$begingroup$
I would await sufficient answers and comments. Then you could self-answer or accept if you please, and perhaps also ask a follow-up question.
$endgroup$
– dfhwze
7 hours ago
1
$begingroup$
The rollback is being discussed on meta.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
6 hours ago
2
2
$begingroup$
Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 3 → 2
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
7 hours ago
$begingroup$
Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 3 → 2
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
7 hours ago
$begingroup$
@SᴀᴍOnᴇᴌᴀ Sorry I did not know. I feel like this was just a small mistake I made which may have caused some misunderstandings. Do I really have to ask a new question just for that (becaues I feel like my main concern didn't have to do with that at all) or how would you recommend me clearing that misunderstanding up to the concerned question?
$endgroup$
– Joelius
7 hours ago
$begingroup$
@SᴀᴍOnᴇᴌᴀ Sorry I did not know. I feel like this was just a small mistake I made which may have caused some misunderstandings. Do I really have to ask a new question just for that (becaues I feel like my main concern didn't have to do with that at all) or how would you recommend me clearing that misunderstanding up to the concerned question?
$endgroup$
– Joelius
7 hours ago
2
2
$begingroup$
I would await sufficient answers and comments. Then you could self-answer or accept if you please, and perhaps also ask a follow-up question.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
I would await sufficient answers and comments. Then you could self-answer or accept if you please, and perhaps also ask a follow-up question.
$endgroup$
– dfhwze
7 hours ago
1
1
$begingroup$
The rollback is being discussed on meta.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
6 hours ago
$begingroup$
The rollback is being discussed on meta.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
6 hours ago
add a comment |
3 Answers
3
active
oldest
votes
$begingroup$
Your method is meant to handle ExpandoObjects
explicitly, so it would be better named ConvertExpandoObjectToDictonary
, and would probably be less confusing if it actually took an ExpandoObject
as the parameter:
Dictionary<string, object> ConvertExpandoObjectToDictonary(ExpandoObject expandoObject);
This is what the public API should look like; you can keep the current method as it is (since you need to call it recursively) but make it private and hide it behind the clearer public API.
This doesn't really make sense:
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
You are looking for a single ExpandoObject
, and then assuming everything you might need is an expando object and ignoring everything else. I think you want something like this instead, which keeps non-expando entries:
return list
.Select(o => o is ExpandoObject
? ConvertDynamicToDictonary((ExpandoObject)o)
: o
);
$endgroup$
$begingroup$
You are enforcing a signature and flow based on a strict interpretation of the spec. Since the spec is lacking some context, I find this approach OK.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze sorry; I'm not sure what you mean? (I'm mostly going by the title, which suggests the purpose of this method is exactly to processExpandoObjects
)
$endgroup$
– VisualMelon
7 hours ago
$begingroup$
The OP uses a dict both to process dictionaries and expando's. You took the ambiguity away. This has an impact on the algorithm, if there really are dictionaries to be processed that aren't expando's. The OP should perhaps make clear the usage of dict.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze I think I see what you mean. I'd maintain that if the intention is to convertExpandoObjects
(which is how I read the OP) then the API should reflect that; the work can be referred to a method that processes dictionaries (per the OP): it needn't have any influence on the algorithm.
$endgroup$
– VisualMelon
7 hours ago
1
$begingroup$
yes :) that would be OK. Let's stop hijacking the comments of this answer now :p
$endgroup$
– dfhwze
7 hours ago
|
show 2 more comments
$begingroup$
Inconsistent definition of ExpandoObject
. It's IDictionary<string, object>
here:
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict); // <- possibly already visited?
}
and ExpandoObject
here:
if (list.Any(o => o is ExpandoObject))
{
// ..
}
I would opt to use IDictionary<string, object>
both to improve consistency and flexibility of handling instances in the hierarchy.
It is not clear whether the structure of the hierarchical data is a tree or a cyclic graph. This has impact on a possible infinite recursion. In case of a cyclic graph, I would keep track of visited references.
Not all sequences are generic classes.
This condition..
if (p.Value is IEnumerable<object> list)
..does not cover when p.Value
is IEnumerable
but not IEnumerable<object>
. As note in another answer, be careful with false positives (like string
).
What if value
is null? Try avoiding null reference exceptions in your code.
$endgroup$
$begingroup$
Thanks for the ideas. The ExpandoObject should always be a tree since it is just an instance of a class with all traces of null properties removed. This is necessary for correctly invoking some Javascript.
$endgroup$
– Joelius
7 hours ago
add a comment |
$begingroup$
A few minor stylistic bits that may increase its utility:
- since you take in an
IDictionary
, return anIDictionary
. - make it
static
as it accesses no instance data or methods. - since it's now
static
, make it an extension method. - validate the parameter passed in.
Also:
- since it's a single
return
statement, make it an expression body.
Further:
- yeah, the
.Any()
is redundant (and less performant if there is anExpandoObject
in the collection) with the.Where()
criteria, so no need for it. - allowed for non-generic
IEnumerable
. - renamed some of the locals to be clearer as to their meaning.
Giving:
static IDictionary<string, object> ConvertDynamicToDictionary(this IDictionary<string, object> source) => source?.ToDictionary(
keySelector => keySelector.Key,
elementSelector =>
{
object value = elementSelector.Value;
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (value is IDictionary<string, object> dictionary)
{
return dictionary.ConvertDynamicToDictionary();
}
// A special case since string implements IEnumerable.
if (value is string stringValue)
{
return stringValue;
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (value is IEnumerable enumerable)
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return enumerable
.Cast<object>()
.Select(element => element is IDictionary<string, object> expando
? expando.ConvertDynamicToDictionary()
: element);
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return value;
});
$endgroup$
$begingroup$
Ugh,string
implementsIEnumerable
.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Clear points, thank you. Yes I already wanted to point out that IEnumerable is used seemingly everywhere and ask if that would cause issues (it would just decrease the performance right?). Also I have edited my question regarding the signature because the method is alreadyprivate
andstatic
I just forgot to write that.
$endgroup$
– Joelius
7 hours ago
$begingroup$
It's not thatIEnumerable
decreases performance, it's that the.Any()
followed by.Where()
with the same lambda does the same checking, and therefore.Any()
is redundant.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
I see. My edit has now been rolled back because I edited the code which you're not supposed to do. My method is actuallyprivate static
- would you still recommend me converting it into an extension method?
$endgroup$
– Joelius
7 hours ago
$begingroup$
I mean, I like them. Especially when the parameter is an interface.
$endgroup$
– Jesse C. Slicer
7 hours ago
|
show 2 more comments
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Joelius is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f223375%2frecursive-conversion-from-expandoobject-to-dictionarystring-object%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Your method is meant to handle ExpandoObjects
explicitly, so it would be better named ConvertExpandoObjectToDictonary
, and would probably be less confusing if it actually took an ExpandoObject
as the parameter:
Dictionary<string, object> ConvertExpandoObjectToDictonary(ExpandoObject expandoObject);
This is what the public API should look like; you can keep the current method as it is (since you need to call it recursively) but make it private and hide it behind the clearer public API.
This doesn't really make sense:
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
You are looking for a single ExpandoObject
, and then assuming everything you might need is an expando object and ignoring everything else. I think you want something like this instead, which keeps non-expando entries:
return list
.Select(o => o is ExpandoObject
? ConvertDynamicToDictonary((ExpandoObject)o)
: o
);
$endgroup$
$begingroup$
You are enforcing a signature and flow based on a strict interpretation of the spec. Since the spec is lacking some context, I find this approach OK.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze sorry; I'm not sure what you mean? (I'm mostly going by the title, which suggests the purpose of this method is exactly to processExpandoObjects
)
$endgroup$
– VisualMelon
7 hours ago
$begingroup$
The OP uses a dict both to process dictionaries and expando's. You took the ambiguity away. This has an impact on the algorithm, if there really are dictionaries to be processed that aren't expando's. The OP should perhaps make clear the usage of dict.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze I think I see what you mean. I'd maintain that if the intention is to convertExpandoObjects
(which is how I read the OP) then the API should reflect that; the work can be referred to a method that processes dictionaries (per the OP): it needn't have any influence on the algorithm.
$endgroup$
– VisualMelon
7 hours ago
1
$begingroup$
yes :) that would be OK. Let's stop hijacking the comments of this answer now :p
$endgroup$
– dfhwze
7 hours ago
|
show 2 more comments
$begingroup$
Your method is meant to handle ExpandoObjects
explicitly, so it would be better named ConvertExpandoObjectToDictonary
, and would probably be less confusing if it actually took an ExpandoObject
as the parameter:
Dictionary<string, object> ConvertExpandoObjectToDictonary(ExpandoObject expandoObject);
This is what the public API should look like; you can keep the current method as it is (since you need to call it recursively) but make it private and hide it behind the clearer public API.
This doesn't really make sense:
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
You are looking for a single ExpandoObject
, and then assuming everything you might need is an expando object and ignoring everything else. I think you want something like this instead, which keeps non-expando entries:
return list
.Select(o => o is ExpandoObject
? ConvertDynamicToDictonary((ExpandoObject)o)
: o
);
$endgroup$
$begingroup$
You are enforcing a signature and flow based on a strict interpretation of the spec. Since the spec is lacking some context, I find this approach OK.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze sorry; I'm not sure what you mean? (I'm mostly going by the title, which suggests the purpose of this method is exactly to processExpandoObjects
)
$endgroup$
– VisualMelon
7 hours ago
$begingroup$
The OP uses a dict both to process dictionaries and expando's. You took the ambiguity away. This has an impact on the algorithm, if there really are dictionaries to be processed that aren't expando's. The OP should perhaps make clear the usage of dict.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze I think I see what you mean. I'd maintain that if the intention is to convertExpandoObjects
(which is how I read the OP) then the API should reflect that; the work can be referred to a method that processes dictionaries (per the OP): it needn't have any influence on the algorithm.
$endgroup$
– VisualMelon
7 hours ago
1
$begingroup$
yes :) that would be OK. Let's stop hijacking the comments of this answer now :p
$endgroup$
– dfhwze
7 hours ago
|
show 2 more comments
$begingroup$
Your method is meant to handle ExpandoObjects
explicitly, so it would be better named ConvertExpandoObjectToDictonary
, and would probably be less confusing if it actually took an ExpandoObject
as the parameter:
Dictionary<string, object> ConvertExpandoObjectToDictonary(ExpandoObject expandoObject);
This is what the public API should look like; you can keep the current method as it is (since you need to call it recursively) but make it private and hide it behind the clearer public API.
This doesn't really make sense:
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
You are looking for a single ExpandoObject
, and then assuming everything you might need is an expando object and ignoring everything else. I think you want something like this instead, which keeps non-expando entries:
return list
.Select(o => o is ExpandoObject
? ConvertDynamicToDictonary((ExpandoObject)o)
: o
);
$endgroup$
Your method is meant to handle ExpandoObjects
explicitly, so it would be better named ConvertExpandoObjectToDictonary
, and would probably be less confusing if it actually took an ExpandoObject
as the parameter:
Dictionary<string, object> ConvertExpandoObjectToDictonary(ExpandoObject expandoObject);
This is what the public API should look like; you can keep the current method as it is (since you need to call it recursively) but make it private and hide it behind the clearer public API.
This doesn't really make sense:
if (list.Any(o => o is ExpandoObject))
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return list
.Where(o => o is ExpandoObject)
.Select(o => ConvertDynamicToDictonary((ExpandoObject)o));
}
You are looking for a single ExpandoObject
, and then assuming everything you might need is an expando object and ignoring everything else. I think you want something like this instead, which keeps non-expando entries:
return list
.Select(o => o is ExpandoObject
? ConvertDynamicToDictonary((ExpandoObject)o)
: o
);
answered 7 hours ago
VisualMelonVisualMelon
4,50612 silver badges29 bronze badges
4,50612 silver badges29 bronze badges
$begingroup$
You are enforcing a signature and flow based on a strict interpretation of the spec. Since the spec is lacking some context, I find this approach OK.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze sorry; I'm not sure what you mean? (I'm mostly going by the title, which suggests the purpose of this method is exactly to processExpandoObjects
)
$endgroup$
– VisualMelon
7 hours ago
$begingroup$
The OP uses a dict both to process dictionaries and expando's. You took the ambiguity away. This has an impact on the algorithm, if there really are dictionaries to be processed that aren't expando's. The OP should perhaps make clear the usage of dict.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze I think I see what you mean. I'd maintain that if the intention is to convertExpandoObjects
(which is how I read the OP) then the API should reflect that; the work can be referred to a method that processes dictionaries (per the OP): it needn't have any influence on the algorithm.
$endgroup$
– VisualMelon
7 hours ago
1
$begingroup$
yes :) that would be OK. Let's stop hijacking the comments of this answer now :p
$endgroup$
– dfhwze
7 hours ago
|
show 2 more comments
$begingroup$
You are enforcing a signature and flow based on a strict interpretation of the spec. Since the spec is lacking some context, I find this approach OK.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze sorry; I'm not sure what you mean? (I'm mostly going by the title, which suggests the purpose of this method is exactly to processExpandoObjects
)
$endgroup$
– VisualMelon
7 hours ago
$begingroup$
The OP uses a dict both to process dictionaries and expando's. You took the ambiguity away. This has an impact on the algorithm, if there really are dictionaries to be processed that aren't expando's. The OP should perhaps make clear the usage of dict.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze I think I see what you mean. I'd maintain that if the intention is to convertExpandoObjects
(which is how I read the OP) then the API should reflect that; the work can be referred to a method that processes dictionaries (per the OP): it needn't have any influence on the algorithm.
$endgroup$
– VisualMelon
7 hours ago
1
$begingroup$
yes :) that would be OK. Let's stop hijacking the comments of this answer now :p
$endgroup$
– dfhwze
7 hours ago
$begingroup$
You are enforcing a signature and flow based on a strict interpretation of the spec. Since the spec is lacking some context, I find this approach OK.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
You are enforcing a signature and flow based on a strict interpretation of the spec. Since the spec is lacking some context, I find this approach OK.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze sorry; I'm not sure what you mean? (I'm mostly going by the title, which suggests the purpose of this method is exactly to process
ExpandoObjects
)$endgroup$
– VisualMelon
7 hours ago
$begingroup$
@dfhwze sorry; I'm not sure what you mean? (I'm mostly going by the title, which suggests the purpose of this method is exactly to process
ExpandoObjects
)$endgroup$
– VisualMelon
7 hours ago
$begingroup$
The OP uses a dict both to process dictionaries and expando's. You took the ambiguity away. This has an impact on the algorithm, if there really are dictionaries to be processed that aren't expando's. The OP should perhaps make clear the usage of dict.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
The OP uses a dict both to process dictionaries and expando's. You took the ambiguity away. This has an impact on the algorithm, if there really are dictionaries to be processed that aren't expando's. The OP should perhaps make clear the usage of dict.
$endgroup$
– dfhwze
7 hours ago
$begingroup$
@dfhwze I think I see what you mean. I'd maintain that if the intention is to convert
ExpandoObjects
(which is how I read the OP) then the API should reflect that; the work can be referred to a method that processes dictionaries (per the OP): it needn't have any influence on the algorithm.$endgroup$
– VisualMelon
7 hours ago
$begingroup$
@dfhwze I think I see what you mean. I'd maintain that if the intention is to convert
ExpandoObjects
(which is how I read the OP) then the API should reflect that; the work can be referred to a method that processes dictionaries (per the OP): it needn't have any influence on the algorithm.$endgroup$
– VisualMelon
7 hours ago
1
1
$begingroup$
yes :) that would be OK. Let's stop hijacking the comments of this answer now :p
$endgroup$
– dfhwze
7 hours ago
$begingroup$
yes :) that would be OK. Let's stop hijacking the comments of this answer now :p
$endgroup$
– dfhwze
7 hours ago
|
show 2 more comments
$begingroup$
Inconsistent definition of ExpandoObject
. It's IDictionary<string, object>
here:
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict); // <- possibly already visited?
}
and ExpandoObject
here:
if (list.Any(o => o is ExpandoObject))
{
// ..
}
I would opt to use IDictionary<string, object>
both to improve consistency and flexibility of handling instances in the hierarchy.
It is not clear whether the structure of the hierarchical data is a tree or a cyclic graph. This has impact on a possible infinite recursion. In case of a cyclic graph, I would keep track of visited references.
Not all sequences are generic classes.
This condition..
if (p.Value is IEnumerable<object> list)
..does not cover when p.Value
is IEnumerable
but not IEnumerable<object>
. As note in another answer, be careful with false positives (like string
).
What if value
is null? Try avoiding null reference exceptions in your code.
$endgroup$
$begingroup$
Thanks for the ideas. The ExpandoObject should always be a tree since it is just an instance of a class with all traces of null properties removed. This is necessary for correctly invoking some Javascript.
$endgroup$
– Joelius
7 hours ago
add a comment |
$begingroup$
Inconsistent definition of ExpandoObject
. It's IDictionary<string, object>
here:
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict); // <- possibly already visited?
}
and ExpandoObject
here:
if (list.Any(o => o is ExpandoObject))
{
// ..
}
I would opt to use IDictionary<string, object>
both to improve consistency and flexibility of handling instances in the hierarchy.
It is not clear whether the structure of the hierarchical data is a tree or a cyclic graph. This has impact on a possible infinite recursion. In case of a cyclic graph, I would keep track of visited references.
Not all sequences are generic classes.
This condition..
if (p.Value is IEnumerable<object> list)
..does not cover when p.Value
is IEnumerable
but not IEnumerable<object>
. As note in another answer, be careful with false positives (like string
).
What if value
is null? Try avoiding null reference exceptions in your code.
$endgroup$
$begingroup$
Thanks for the ideas. The ExpandoObject should always be a tree since it is just an instance of a class with all traces of null properties removed. This is necessary for correctly invoking some Javascript.
$endgroup$
– Joelius
7 hours ago
add a comment |
$begingroup$
Inconsistent definition of ExpandoObject
. It's IDictionary<string, object>
here:
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict); // <- possibly already visited?
}
and ExpandoObject
here:
if (list.Any(o => o is ExpandoObject))
{
// ..
}
I would opt to use IDictionary<string, object>
both to improve consistency and flexibility of handling instances in the hierarchy.
It is not clear whether the structure of the hierarchical data is a tree or a cyclic graph. This has impact on a possible infinite recursion. In case of a cyclic graph, I would keep track of visited references.
Not all sequences are generic classes.
This condition..
if (p.Value is IEnumerable<object> list)
..does not cover when p.Value
is IEnumerable
but not IEnumerable<object>
. As note in another answer, be careful with false positives (like string
).
What if value
is null? Try avoiding null reference exceptions in your code.
$endgroup$
Inconsistent definition of ExpandoObject
. It's IDictionary<string, object>
here:
if (p.Value is IDictionary<string, object> dict)
{
return ConvertDynamicToDictonary(dict); // <- possibly already visited?
}
and ExpandoObject
here:
if (list.Any(o => o is ExpandoObject))
{
// ..
}
I would opt to use IDictionary<string, object>
both to improve consistency and flexibility of handling instances in the hierarchy.
It is not clear whether the structure of the hierarchical data is a tree or a cyclic graph. This has impact on a possible infinite recursion. In case of a cyclic graph, I would keep track of visited references.
Not all sequences are generic classes.
This condition..
if (p.Value is IEnumerable<object> list)
..does not cover when p.Value
is IEnumerable
but not IEnumerable<object>
. As note in another answer, be careful with false positives (like string
).
What if value
is null? Try avoiding null reference exceptions in your code.
edited 7 hours ago
answered 7 hours ago
dfhwzedfhwze
3,9191 gold badge6 silver badges32 bronze badges
3,9191 gold badge6 silver badges32 bronze badges
$begingroup$
Thanks for the ideas. The ExpandoObject should always be a tree since it is just an instance of a class with all traces of null properties removed. This is necessary for correctly invoking some Javascript.
$endgroup$
– Joelius
7 hours ago
add a comment |
$begingroup$
Thanks for the ideas. The ExpandoObject should always be a tree since it is just an instance of a class with all traces of null properties removed. This is necessary for correctly invoking some Javascript.
$endgroup$
– Joelius
7 hours ago
$begingroup$
Thanks for the ideas. The ExpandoObject should always be a tree since it is just an instance of a class with all traces of null properties removed. This is necessary for correctly invoking some Javascript.
$endgroup$
– Joelius
7 hours ago
$begingroup$
Thanks for the ideas. The ExpandoObject should always be a tree since it is just an instance of a class with all traces of null properties removed. This is necessary for correctly invoking some Javascript.
$endgroup$
– Joelius
7 hours ago
add a comment |
$begingroup$
A few minor stylistic bits that may increase its utility:
- since you take in an
IDictionary
, return anIDictionary
. - make it
static
as it accesses no instance data or methods. - since it's now
static
, make it an extension method. - validate the parameter passed in.
Also:
- since it's a single
return
statement, make it an expression body.
Further:
- yeah, the
.Any()
is redundant (and less performant if there is anExpandoObject
in the collection) with the.Where()
criteria, so no need for it. - allowed for non-generic
IEnumerable
. - renamed some of the locals to be clearer as to their meaning.
Giving:
static IDictionary<string, object> ConvertDynamicToDictionary(this IDictionary<string, object> source) => source?.ToDictionary(
keySelector => keySelector.Key,
elementSelector =>
{
object value = elementSelector.Value;
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (value is IDictionary<string, object> dictionary)
{
return dictionary.ConvertDynamicToDictionary();
}
// A special case since string implements IEnumerable.
if (value is string stringValue)
{
return stringValue;
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (value is IEnumerable enumerable)
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return enumerable
.Cast<object>()
.Select(element => element is IDictionary<string, object> expando
? expando.ConvertDynamicToDictionary()
: element);
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return value;
});
$endgroup$
$begingroup$
Ugh,string
implementsIEnumerable
.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Clear points, thank you. Yes I already wanted to point out that IEnumerable is used seemingly everywhere and ask if that would cause issues (it would just decrease the performance right?). Also I have edited my question regarding the signature because the method is alreadyprivate
andstatic
I just forgot to write that.
$endgroup$
– Joelius
7 hours ago
$begingroup$
It's not thatIEnumerable
decreases performance, it's that the.Any()
followed by.Where()
with the same lambda does the same checking, and therefore.Any()
is redundant.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
I see. My edit has now been rolled back because I edited the code which you're not supposed to do. My method is actuallyprivate static
- would you still recommend me converting it into an extension method?
$endgroup$
– Joelius
7 hours ago
$begingroup$
I mean, I like them. Especially when the parameter is an interface.
$endgroup$
– Jesse C. Slicer
7 hours ago
|
show 2 more comments
$begingroup$
A few minor stylistic bits that may increase its utility:
- since you take in an
IDictionary
, return anIDictionary
. - make it
static
as it accesses no instance data or methods. - since it's now
static
, make it an extension method. - validate the parameter passed in.
Also:
- since it's a single
return
statement, make it an expression body.
Further:
- yeah, the
.Any()
is redundant (and less performant if there is anExpandoObject
in the collection) with the.Where()
criteria, so no need for it. - allowed for non-generic
IEnumerable
. - renamed some of the locals to be clearer as to their meaning.
Giving:
static IDictionary<string, object> ConvertDynamicToDictionary(this IDictionary<string, object> source) => source?.ToDictionary(
keySelector => keySelector.Key,
elementSelector =>
{
object value = elementSelector.Value;
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (value is IDictionary<string, object> dictionary)
{
return dictionary.ConvertDynamicToDictionary();
}
// A special case since string implements IEnumerable.
if (value is string stringValue)
{
return stringValue;
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (value is IEnumerable enumerable)
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return enumerable
.Cast<object>()
.Select(element => element is IDictionary<string, object> expando
? expando.ConvertDynamicToDictionary()
: element);
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return value;
});
$endgroup$
$begingroup$
Ugh,string
implementsIEnumerable
.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Clear points, thank you. Yes I already wanted to point out that IEnumerable is used seemingly everywhere and ask if that would cause issues (it would just decrease the performance right?). Also I have edited my question regarding the signature because the method is alreadyprivate
andstatic
I just forgot to write that.
$endgroup$
– Joelius
7 hours ago
$begingroup$
It's not thatIEnumerable
decreases performance, it's that the.Any()
followed by.Where()
with the same lambda does the same checking, and therefore.Any()
is redundant.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
I see. My edit has now been rolled back because I edited the code which you're not supposed to do. My method is actuallyprivate static
- would you still recommend me converting it into an extension method?
$endgroup$
– Joelius
7 hours ago
$begingroup$
I mean, I like them. Especially when the parameter is an interface.
$endgroup$
– Jesse C. Slicer
7 hours ago
|
show 2 more comments
$begingroup$
A few minor stylistic bits that may increase its utility:
- since you take in an
IDictionary
, return anIDictionary
. - make it
static
as it accesses no instance data or methods. - since it's now
static
, make it an extension method. - validate the parameter passed in.
Also:
- since it's a single
return
statement, make it an expression body.
Further:
- yeah, the
.Any()
is redundant (and less performant if there is anExpandoObject
in the collection) with the.Where()
criteria, so no need for it. - allowed for non-generic
IEnumerable
. - renamed some of the locals to be clearer as to their meaning.
Giving:
static IDictionary<string, object> ConvertDynamicToDictionary(this IDictionary<string, object> source) => source?.ToDictionary(
keySelector => keySelector.Key,
elementSelector =>
{
object value = elementSelector.Value;
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (value is IDictionary<string, object> dictionary)
{
return dictionary.ConvertDynamicToDictionary();
}
// A special case since string implements IEnumerable.
if (value is string stringValue)
{
return stringValue;
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (value is IEnumerable enumerable)
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return enumerable
.Cast<object>()
.Select(element => element is IDictionary<string, object> expando
? expando.ConvertDynamicToDictionary()
: element);
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return value;
});
$endgroup$
A few minor stylistic bits that may increase its utility:
- since you take in an
IDictionary
, return anIDictionary
. - make it
static
as it accesses no instance data or methods. - since it's now
static
, make it an extension method. - validate the parameter passed in.
Also:
- since it's a single
return
statement, make it an expression body.
Further:
- yeah, the
.Any()
is redundant (and less performant if there is anExpandoObject
in the collection) with the.Where()
criteria, so no need for it. - allowed for non-generic
IEnumerable
. - renamed some of the locals to be clearer as to their meaning.
Giving:
static IDictionary<string, object> ConvertDynamicToDictionary(this IDictionary<string, object> source) => source?.ToDictionary(
keySelector => keySelector.Key,
elementSelector =>
{
object value = elementSelector.Value;
// if it's another IDict (might be a ExpandoObject or could also be an actual Dict containing ExpandoObjects) just go through it recursively
if (value is IDictionary<string, object> dictionary)
{
return dictionary.ConvertDynamicToDictionary();
}
// A special case since string implements IEnumerable.
if (value is string stringValue)
{
return stringValue;
}
// if it's an IEnumerable, it might have ExpandoObjects inside, so check for that
if (value is IEnumerable enumerable)
{
// if it does contain ExpandoObjects, take all of those and also go through them recursively
return enumerable
.Cast<object>()
.Select(element => element is IDictionary<string, object> expando
? expando.ConvertDynamicToDictionary()
: element);
}
// neither an IDict nor an IEnumerable -> it's probably fine to just return the value it has
return value;
});
edited 7 hours ago
answered 7 hours ago
Jesse C. SlicerJesse C. Slicer
11.6k27 silver badges40 bronze badges
11.6k27 silver badges40 bronze badges
$begingroup$
Ugh,string
implementsIEnumerable
.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Clear points, thank you. Yes I already wanted to point out that IEnumerable is used seemingly everywhere and ask if that would cause issues (it would just decrease the performance right?). Also I have edited my question regarding the signature because the method is alreadyprivate
andstatic
I just forgot to write that.
$endgroup$
– Joelius
7 hours ago
$begingroup$
It's not thatIEnumerable
decreases performance, it's that the.Any()
followed by.Where()
with the same lambda does the same checking, and therefore.Any()
is redundant.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
I see. My edit has now been rolled back because I edited the code which you're not supposed to do. My method is actuallyprivate static
- would you still recommend me converting it into an extension method?
$endgroup$
– Joelius
7 hours ago
$begingroup$
I mean, I like them. Especially when the parameter is an interface.
$endgroup$
– Jesse C. Slicer
7 hours ago
|
show 2 more comments
$begingroup$
Ugh,string
implementsIEnumerable
.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Clear points, thank you. Yes I already wanted to point out that IEnumerable is used seemingly everywhere and ask if that would cause issues (it would just decrease the performance right?). Also I have edited my question regarding the signature because the method is alreadyprivate
andstatic
I just forgot to write that.
$endgroup$
– Joelius
7 hours ago
$begingroup$
It's not thatIEnumerable
decreases performance, it's that the.Any()
followed by.Where()
with the same lambda does the same checking, and therefore.Any()
is redundant.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
I see. My edit has now been rolled back because I edited the code which you're not supposed to do. My method is actuallyprivate static
- would you still recommend me converting it into an extension method?
$endgroup$
– Joelius
7 hours ago
$begingroup$
I mean, I like them. Especially when the parameter is an interface.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Ugh,
string
implements IEnumerable
.$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Ugh,
string
implements IEnumerable
.$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
Clear points, thank you. Yes I already wanted to point out that IEnumerable is used seemingly everywhere and ask if that would cause issues (it would just decrease the performance right?). Also I have edited my question regarding the signature because the method is already
private
and static
I just forgot to write that.$endgroup$
– Joelius
7 hours ago
$begingroup$
Clear points, thank you. Yes I already wanted to point out that IEnumerable is used seemingly everywhere and ask if that would cause issues (it would just decrease the performance right?). Also I have edited my question regarding the signature because the method is already
private
and static
I just forgot to write that.$endgroup$
– Joelius
7 hours ago
$begingroup$
It's not that
IEnumerable
decreases performance, it's that the .Any()
followed by .Where()
with the same lambda does the same checking, and therefore .Any()
is redundant.$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
It's not that
IEnumerable
decreases performance, it's that the .Any()
followed by .Where()
with the same lambda does the same checking, and therefore .Any()
is redundant.$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
I see. My edit has now been rolled back because I edited the code which you're not supposed to do. My method is actually
private static
- would you still recommend me converting it into an extension method?$endgroup$
– Joelius
7 hours ago
$begingroup$
I see. My edit has now been rolled back because I edited the code which you're not supposed to do. My method is actually
private static
- would you still recommend me converting it into an extension method?$endgroup$
– Joelius
7 hours ago
$begingroup$
I mean, I like them. Especially when the parameter is an interface.
$endgroup$
– Jesse C. Slicer
7 hours ago
$begingroup$
I mean, I like them. Especially when the parameter is an interface.
$endgroup$
– Jesse C. Slicer
7 hours ago
|
show 2 more comments
Joelius is a new contributor. Be nice, and check out our Code of Conduct.
Joelius is a new contributor. Be nice, and check out our Code of Conduct.
Joelius is a new contributor. Be nice, and check out our Code of Conduct.
Joelius is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f223375%2frecursive-conversion-from-expandoobject-to-dictionarystring-object%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
2
$begingroup$
Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 3 → 2
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
7 hours ago
$begingroup$
@SᴀᴍOnᴇᴌᴀ Sorry I did not know. I feel like this was just a small mistake I made which may have caused some misunderstandings. Do I really have to ask a new question just for that (becaues I feel like my main concern didn't have to do with that at all) or how would you recommend me clearing that misunderstanding up to the concerned question?
$endgroup$
– Joelius
7 hours ago
2
$begingroup$
I would await sufficient answers and comments. Then you could self-answer or accept if you please, and perhaps also ask a follow-up question.
$endgroup$
– dfhwze
7 hours ago
1
$begingroup$
The rollback is being discussed on meta.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
6 hours ago