patterncsharpMinor
Improve this reflection bashing code
Viewed 0 times
thisreflectionbashingimprovecode
Problem
I have implemented an
The code is a bit wild using reflection and compiling of delegates. Are there any subtle improvements or obvious over engineering problems here?
This is a follow-up from a Stack Overflow question, but further improvements feel like code review rather than specific questions.
TESTCASE
```
using FluentAssertions;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Runtime.Serialization;
using System.Text;
using System.Xml;
using Xunit;
namespace ReactiveUI.Ext.Spec
{
[DataContract(Name="Node", Namespace="http://foo.com/")]
class Node
{
[DataMember()]
public string Name;
}
[DataContract(Name="Fixture", Namespace="http://foo.com/")]
class FixtureType
{
[DataMember()]
public ImmutableList Nodes;
public FixtureType(){
Nodes = ImmutableList.Empty.AddRange( new []
{ new Node(){Name="A"}
, new Node(){Name="B"}
, new Node(){Name="C"}
});
}
}
public class ImmutableSurrogateSpec
{
public static string ToXML(object obj)
{
var settings = new XmlWriterSettings { Indent = true };
using (MemoryStream memoryStream = new MemoryStream())
using (StreamReader reader = new StreamReader(memoryStream))
using (XmlWriter writer = XmlWriter.Create(memoryStream, settings))
{
DataContractSerializer serializer =
new DataContractSerializer
( obj.GetType()
, new DataContractSerializerSettings() { DataContractSurrogate = new ImmutableSurrogateSerializer() }
);
serializer.WriteObject(writer, o
IDataContractSurrogate to enable serialization of ImmutableList (from the Microsoft Immutable Collections BCL) using the DataContractSerialization framework.The code is a bit wild using reflection and compiling of delegates. Are there any subtle improvements or obvious over engineering problems here?
This is a follow-up from a Stack Overflow question, but further improvements feel like code review rather than specific questions.
TESTCASE
```
using FluentAssertions;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Runtime.Serialization;
using System.Text;
using System.Xml;
using Xunit;
namespace ReactiveUI.Ext.Spec
{
[DataContract(Name="Node", Namespace="http://foo.com/")]
class Node
{
[DataMember()]
public string Name;
}
[DataContract(Name="Fixture", Namespace="http://foo.com/")]
class FixtureType
{
[DataMember()]
public ImmutableList Nodes;
public FixtureType(){
Nodes = ImmutableList.Empty.AddRange( new []
{ new Node(){Name="A"}
, new Node(){Name="B"}
, new Node(){Name="C"}
});
}
}
public class ImmutableSurrogateSpec
{
public static string ToXML(object obj)
{
var settings = new XmlWriterSettings { Indent = true };
using (MemoryStream memoryStream = new MemoryStream())
using (StreamReader reader = new StreamReader(memoryStream))
using (XmlWriter writer = XmlWriter.Create(memoryStream, settings))
{
DataContractSerializer serializer =
new DataContractSerializer
( obj.GetType()
, new DataContractSerializerSettings() { DataContractSurrogate = new ImmutableSurrogateSerializer() }
);
serializer.WriteObject(writer, o
Solution
This actually looks pretty nice and I am bookmarking it. :)
Two tiny details, which are probably a waste of time:
-
I would personally (probably) make the
-
I usually try to avoid looking up a hash table multiple times, even though the performance difference is usually negligible. In your case,
But as I've written, apart from that, it's a really nice approach. Proper de/serialization of immutable types is something I've been looking for since .NET 2.0 and the plain old
Two tiny details, which are probably a waste of time:
-
I would personally (probably) make the
ImmutableListListConverter a private class inside of the ImmutableListListConverter, since it's an implementation detail of the latter class. -
I usually try to avoid looking up a hash table multiple times, even though the performance difference is usually negligible. In your case,
ImmutableSurrogateSerializer.GetDataContractType could probably avoid looking at the dictionary most of the time by doing the if guard at the beginning and then doing the lookup/insertion in a single step. Also, the indexer right after ContainsKey results in that unnecessary second lookup. Note that this is what I find simpler to read, not because the performance difference is anything but negligible:public Type GetDataContractType(Type t)
{
// if this is not an immutable list, we can just skip the rest immediately
if (!t.IsGenericType || t.GetGenericTypeDefinition() != typeof(ImmutableList<>))
{
return type;
}
// get the result through the cache
return _TypeCache.GetOrAdd(t, t => typeof(List<>).MakeGenericType(t.GetGenericArguments()));
}But as I've written, apart from that, it's a really nice approach. Proper de/serialization of immutable types is something I've been looking for since .NET 2.0 and the plain old
XmlSerializer.Code Snippets
public Type GetDataContractType(Type t)
{
// if this is not an immutable list, we can just skip the rest immediately
if (!t.IsGenericType || t.GetGenericTypeDefinition() != typeof(ImmutableList<>))
{
return type;
}
// get the result through the cache
return _TypeCache.GetOrAdd(t, t => typeof(List<>).MakeGenericType(t.GetGenericArguments()));
}Context
StackExchange Code Review Q#31688, answer score: 2
Revisions (0)
No revisions yet.