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;
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!).