Different Kind Of Factory?
Posted by Davy Brion on March 4th, 2009
I was just refactoring some code and there were a few places in a class where we needed to instantiate a new instance of something, based on a key… could’ve used a factory class there, right? Instead i did this:
private readonly Dictionary<string, Func<IPanel>> panelFactory = new Dictionary<string, Func<IPanel>>
{
{ typeAndSizeKey, () => new TypeAndSizePanel() },
{ phaseKey, () => new PhasePanel() },
{ organisationStructureKey, () => new OrganisationStructurePanel() },
{ incidentDataKey, () => new IncidentDataPanel() },
{ processesKey, () => new ProcessesPanel() },
{ generalInfoKey, () => new GeneralInfoPanel() },
{ linksKey, () => new LinksPanel() }
};
and now, when i need to create a new instance i just do this:
panelFactory[myKey]()
i kinda like the simplicity

March 4th, 2009 at 1:19 pm
I’m a bit rusty with lambdas and all that… this doesn’t create an instance until you call the appropriate key, right?
March 4th, 2009 at 1:28 pm
The Dictionary returns the appropriate lambda (well, the Func actually) which is associated with the given key… only when you execute the return value of the dictionary is the lambda actually executed.
So yes, it only instantiates the objects when you retrieve the correct func and execute it… you could also use it like this:
var myCreationFunction = panelFactory[myKey];
myCreationFunction(); // this actually instantiates the panel
March 4th, 2009 at 6:48 pm
Very nice
March 4th, 2009 at 8:27 pm
I recently used similar code. Very cool. This works great when you only need to create objects within a single class. If changes are made and at least one outside class suddenly needs to access that Dictionary object, it gets to be a small pain when using “magic strings.” It feels safer using an enum as the key or refactoring to a typical factory class. (Small point, but obvious.) Also, if you have a ton of potential keys, an internally accessible enum is a better way to go (IMHO). Intellisense = godsend.
March 4th, 2009 at 8:32 pm
yes, one of my coworkers (hi peter!) asked me why i didn’t use an enum, so i eventually caved and started using an enum as the key
to be honest, it was mostly out of laziness why i didn’t use an enum in the first place
March 4th, 2009 at 8:36 pm
You bake the cake, I put the little cherry on top!
March 4th, 2009 at 8:48 pm
@Peter,
that comment just cost us our ‘coolness’-factor
March 4th, 2009 at 9:50 pm
[...] Different Kind Of Factory? – Davy Brion [...]
March 5th, 2009 at 5:41 pm
If you know enough about what type of object you need to be able to specify a “key” to the factory, don’t you know enough to just call the constructor of the class?
March 5th, 2009 at 8:22 pm
@Stew,
that was exactly what i was trying to get away from in this class… these panels were instantiated whenever they were needed (which was in 2 situations) and i just didn’t like it. Also, the code that uses these panels only needs to have an IPanel instance, it doesn’t really care about what specific panel it is.
March 5th, 2009 at 9:26 pm
@Davy,
I’m wondering if you find some advantage in using this method in the two situations where you replaced the direct constructor calls? Obviously, the need for IPanel is controlled by the class to which you are passing the concrete instance.
It seems to me that you would have some clear advantages by calling the constructor directly, such as the ability to discover usages of the class and compiler errors in any places where you may be using properties/methods specific to the concrete implementation.
Are you realizing some benefits that offset these concessions?
March 5th, 2009 at 9:40 pm
the class i used this in has to manage the currently visible panels… this means it has to create the initial panel instances, but it also occasionally has to replace a panel with a new instance. Deciding which specific panel type to instantiate was one ugly switch statement, plus the instantiation statements were in 2 separate locations.
Now, the code that deals with replacing panels doesn’t even need to know anything about which type of panel it needs to remove and create again. It receives a value (previously the string key, but that’s already been changed to an enum value) from an external component, and uses the dictionary-based factory to create a new instance and replace the previous instance (which is also stored in a dictionary based on the key/enum value) with the new one. What used to be a switch statement has now become something like: SwitchPanel(key, panelFactory[key]()) which in turn can do something like:
var previousPanel = currentPanels[key];
previousPanel.Dispose();
currentPanels[key] = newPanel;
The only place where the actual types of the panels are used is in the dictionary-based factory, nowhere else
March 6th, 2009 at 12:07 am
Not using Windsor in your project?
March 6th, 2009 at 12:10 am
@Bill
not in the silverlight code
we’re actually using a lightweight custom IOC container implementation in our silverlight code, but i use it for ‘real’ code, not the UI stuff
the server-side business layer uses Windsor all the way of course, i wouldn’t have it any other way
March 6th, 2009 at 6:53 am
To be honest I don’t see this as anything other than abuse of syntactic sugar resulting in less readable code. How is this any better than a function with a switch statement?
The “Coolness” factor isn’t justification for use of languages features. Where’s the value-add?
Cheers
OJ
March 6th, 2009 at 11:00 am
@OJ
i guess it kinda depends on how you feel about switch statements… personally i try to avoid them as much as possible. As for readability, i’ll take the dictionary over a switch any day as well.
March 6th, 2009 at 4:55 pm
@OJ
I understand everyone has their own opinion, but for me, something like this helps create concise and readable code. That’s all. “Syntactic sugar” gets thrown around quite a bit these days as if it’s a bad thing. I would say this is the use of syntactic sugar, not an abuse of it. I see this kind of code for operations that are class-internal and isolated. It’s quick to write, the intent is easy to understand, and it’s useful. Also, it’s pretty easy to refactor if its use explodes. No big deal.
@Davy
Silverlight, eh? Using the MVVM pattern (aka WPF/SL-specific Presentation Model pattern) that’s gaining popularity? Prism? Caliburn? Whatever you’re using, I wouldn’t mind reading a post about it in the near future, if I might make a suggestion.
March 6th, 2009 at 5:30 pm
@Kevin
we’re not using Prism or Caliburn (though i have been meaning to look into those). We’re currently using a custom solution which is based on my ASP.NET MVP approach with some extra’s thrown in for databinding.
March 7th, 2009 at 12:08 pm
Thanks for the responses guys. I don’t exactly have a problem with the idea of a dictionary. The gratuitous lambdas which result in statements like this:
panelFactory[myKey]().. is really where my issue lies. It’s not immediately obvious what’s going on there. I guess I personally don’t see how this is actually any different to a switch (other than arguably slower?).
Each to their own, so long as I don’t have to maintain it
Cheers lads!
March 8th, 2009 at 12:40 pm
@OJ
If you needed to do what is being done here how would you go about rewriting it?
Personally if improving readability is the aim (which of course is a key factor) I would create an actual factory which had an interface which I could add registrations to and then have a method by which I could retrieve registrations based on a key value. Under neither, the structure could do anything (i.e. use a dictionary, switch statement, or something else like an IoC).
But in if this pattern was going to be common in my code I probably would use an IoC.
March 8th, 2009 at 11:50 pm
Good question John
Time for me to show my hand eh? Let me just go dig one up.
To be honest, I think it would vary depending on what was going on. But on the surface I would do exactly what you described in your last comment, though I might not go as far as having a registration mechanism. A simple switch based on type might be all that’s needed.
The crux of my “gripe” (for want of a better word) is that I find the manner in which the code has to be invoked rather unintuitive. If you wrapped up your current mechanism in a factory function I’d be a lot more comfortable with it. You’d have a simple function call through your code instead of foo[bar]();
Having said all that, your post is an interesting snippet of how you might use lambdas in this case, but just coz you can doesn’t mean you should
Thanks again
Cheers!
OJ