Properly instantiating a class assigned to a private static mutable variable using reflection and lock

So, a contrived example of what I want to improve or confirm.

I use (my / i) BATIS.NET (lightweight ORM / data mapper structure) and I have a class with a static link to each of the table tablets for the database. It works great, but there are so many repetitions that I thought there might be an opportunity to greatly simplify the code. The class looks like this:

public sealed class MyRepository { private static string _connectionString; private volatile static TableAbcMapper _tableAbcMapper; private volatile static TableXyzMapper _tableXyzMapper; // and about 30 more of these private MyRepository() { } public static void Init(string connectionString) { _connectionString = connectionString; } public static string ConnectionString { get { return _connectionString; } } public static TableAbcMapper TableAbc { get { if (_tableAbcMapper == null) { lock (typeof(TableAbcMapper)) { if (_tableAbcMapper == null) { _tableAbcMapper = new TableAbcMapper(_connectionString); } } } return _tableAbcMapper; } } public static TableXyzMapper TableXyz { get { if (_tableXyzMapper == null) { lock (typeof(TableXyzMapper)) { if (_tableXyzMapper == null) { _tableXyzMapper = new TableXyzMapper(_connectionString); } } } return _tableXyzMapper; } } // and about 30 more of these readonly properties } 

Every time I add or delete a table to the database, I can add a private volatile static field and this is a big ugly singleton-y property in the MyRepository class. My first idea was to force properties to call the generic instancing function inside the class; something similar to:

 private static void InitMapper<TMapper>(TMapper instance) where TMapper : MyMapper { lock (typeof(TMapper)) { if (instance == null) { instance = Activator.CreateInstance(typeof(TMapper), new object[] { _connectionString }) as TMapper; } } } 

Then public getters can be slightly reduced to:

 public static TableXyzMapper TableXyz { get { if (_tableXyzMapper == null) { InitMapper<TableXyzMapper>(_tableXyzMapper); } return _tableXyzMapper; } } 

But I don’t know if such a great walk around volatile fields, and using ref or out with volatile fields is no-no, and, among other things, it does not reduce the amount of code very much.

What I would like to do is completely reorganize the MyRepository class, so that it has no private fields and no public getters, and it uses reflection to immediately initialize all the mappers, rather than lazily load them. I would not have to change any code that uses the MyRepository class, since it will look exactly the same, but under the hood it will be a little different:

 public sealed class MyRepository { private MyRepository() { } public volatile static TableAbcMapper TableAbc = null; public volatile static TableXyzMapper TableXyz = null; public static void Init(string connectionString) { foreach (var fieldInfo in typeof(MyRepository).GetFields(BindingFlags.Static)) { if (fieldInfo.GetValue(new MyRepository()) == null) { lock (fieldInfo.FieldType) { if (fieldInfo.GetValue(new MyRepository()) == null) { fieldInfo.SetValue(new MyRepository(), fieldInfo.FieldType.GetConstructor(new Type[] { typeof(string) }) .Invoke(new object[] { connectionString })); } } } } } } 

Now, the only service I have to do when new databases are added to the database is to add a new public volatile static field to it, and the reflections will take care of the rest.

A few questions that I have with this approach:

  • Is this approach functionally equivalent to the original class?
  • Is there any danger to determining volatility variables using reflection?
  • How readable is it, as the source class (assuming all this is commented out)?

Finally, if this is a question that is best suited for the Code Review site, I am all for porting it (mods!).

+4
source share
1 answer

This may not be much shorter, but since you already have an init method, you can create a lazy value that is created on first access. The good thing about Lazy (part of .NET 4) is that you can specify that a value can be created more than once, but its value is published only once (gives the best perfection).

 class Program { static Lazy<string> _Lazy; static string _connectionString; public string LazyValue { get { return _Lazy.Value; } } public static void Init(string connectionString) { _connectionString = connectionString; _Lazy = new Lazy<string>(() => new string(connectionString.ToArray()), System.Threading.LazyThreadSafetyMode.ExecutionAndPublication); } 

It will not be much shorter though.

+1
source

All Articles