Scott Hanselman

Learning WPF with BabySmash - Factories, Interfaces, Delegates and Lambdas, oh my!

June 13, '08 Comments [19] Posted in BabySmash | Windows Client | WPF
Sponsored By

NOTE: If you haven't read the first post in this series, I would encourage you do to that first, or check out the BabySmash category. Also check out http://windowsclient.net/ for more developer info on WPF.

BACKGROUND: This is one of a series of posts on learning WPF. I wrote an application for my 2 year old using WPF, but as I'm a Win32-minded programmer, my working app is full of Win32-isms. It's not a good example of a WPF application even though it uses the technology. I'm calling on community (that's you, Dear Reader) to blog about your solutions to different (horrible) selections of my code. You can get the code http://www.codeplex.com/babysmash. Post your solutions on your blog, in the comments, or in the Issue Tracker and we'll all learn WPF together. Pick a single line, a section, or subsystem or the whole app!

BabySmash has a little mini-hierarchy of "Figures" that represent the Shapes on the screen. There is a "FigureGenerator" that is meant to return a random Figure whenever the baby hits a key.

I thought it would be interesting to walk through the several different ways one could solve this problem, depending on what version of the .NET Framework one has available. This isn't an exhaustive list, of course, and I'm VERY interested in your refactorings, Dear Reader, so leave them in the comments inside <pre> tags. Thanks to Scott Cate (and his son) for his help with this post today.

Original Yucky Random Factory

What's the catch? Well, here's one. The "FigureGenerator" looks like this currently. I know it sucks because of all the TODOs. If that isn't enough of a hint for you, the switch statement is your 9th hint. This is actually pretty common WTF code that you've likely seen in the wild. Perhaps you'd even admit to writing something like it yourself.

private static Figure GenerateFigure(string letter)
{
//TODO: Should this be in XAML? Would that make it better?
Brush fill = Utils.GetRandomColoredBrush();
if (letter.Length == 1 && Char.IsLetterOrDigit(letter[0]))
{
return new LetterFigure(fill, letter);
}

int shape = Utils.RandomBetweenTwoNumbers(0, 6);
//TODO: Should I change the height, width and stroke to be relative to the screen size?
//TODO: I think I need a shapefactory?
//TODO: Where can I get REALLY complex shapes like animal vectors or custom pics? Where do I store them?
switch (shape)
{
case 0:
return new SquareFigure(fill);
case 1:
return new CircleFigure(fill);
case 2:
return new TriangleFigure(fill);
case 3:
return new StarFigure(fill);
case 4:
return new HeartFigure(fill);
case 5:
return new TrapezoidFigure(fill);
case 6:
return new RectangleFigure(fill);
}
return null;
}

Why so gross? Well, the goal here is to be an object factory, but to be a random object factory that returns any of a list of Figures. Notice, though, that a Letter is a special kind of Figure depending on what key the baby pressed. It's still a figure, except it's the only one that cares what key was pressed.

What I need is a better Factory to make these Figures, but a "random factory" that is a little easier to extend than a Switch statement.

Random List of Factories with Interfaces - The 1.1 Way

I'm trying to create a RandomFactory that makes concrete instances of random Shapes that drive from an abstract class. Its the randomness that's important, so I need to randomly select from a list of potentials.

On the reflection side of things, I could have created an array of System.Types then randomly picked one, then used Reflection via Activator.CreateInstance to create something concrete. However, Reflection should be a last resort, and this isn't that complex of a pattern.

One oldish, simple solution would be to create an array of FigureFactories like this:

interface IFigureFactory
{
Figure CreateFigure(Brush fill);
}

public class StarFigureFactory : IFigureFactory
{
public Figure CreateFigure(Brush fill)
{
return new StarFigure(fill);
}
}
public class SquareFigureFactory : IFigureFactory
{
public Figure CreateFigure(Brush fill)
{
return new SquareFigure(fill);
}
}

I'd need to make an array of these, then pick one randomly:

IFigureFactory[] figfacts = new IFigureFactory[]
{
new StarFigureFactory(),
new SquareFigureFactory()
//etc...
};
IFigureFactory figfac = Utils.RandomBetweenTwoNumbers(0, figfacts.Length);
Figure f = figfac.CreateFigure(fill);

This works pretty well, but requires all those FigureFactories.

Random List of Factories with Generics - The 2.0 Second Way Attempt

But, if I was using .NET 2.0, I might say, "hey, what about generics?" I could start creating a GenericFigureFactory<T> to try and get rid of all those other custom tiny factories.

public class GenericFigureFactory<T> where T : Figure, new() 
{
public Figure CreateFigure(Brush fill)
{
Figure f = new T(fill);
//Cannot provide arguments when creating instance of T
}
}

However, it's not so clean because even though the system KNOWS that T is of type Figure and that Figure has a constructor that takes a Brush, I can't call that constructor. At this point, I'd need to set properties instead of a constructor with a parameter. Poop. FAIL.

Random Factories with Delegates - The 2.0 Way

Perhaps this is too much work, with the generics and the interfaces. All I really need is a bunch of methods where the signature is "take a Brush, return a Figure." Some kind of delegate that had that signature. ;)

public delegate Figure FigureFactory<Brush, Figure>(Brush x);

So that's 2.0 style delegate that takes a Brush and returns a Factory. Remember that a delegate in this example an exemplar. It's an example of what a method should look like. It's not the method itself, just the meta-method.

I can make a list of these factories, defining the body as I add them. Then, grab a random one and call it.

List<FigureFactory<Brush, Figure>> foo = new List<FigureFactory<Brush, Figure>>();
foo.Add(delegate(Brush x) { return new SquareFigure(x); });
foo.Add(delegate(Brush x) { return new StarFigure(x); });
FigureFactory myFunc = foo[Utils.RandomBetweenTwoNumbers(0, foo.Count-1)];
Figure f = myFunc(fill);

That delegate up there that takes something and returns something could be made generic, like this:

public delegate TResult Func<TParam, TResult>(TParam p);

That's SO useful that it was added in .NET 3.5 as System.Func and it lives in System.Core.dll. If you want this kind of functionality in .NET 2.0, then just define your own bunch of Funcs. I got this tip from Scott Cate. Remember, if you're using .NET 3.5, you don't need these, they already exist.

public delegate void Func<T>();
public delegate TRes Func<TSrc1, TRes>(TSrc1 src1);
public delegate TRes Func<TSrc1, TSrc2, TRes>(TSrc1 src1, TSrc2 src2);
public delegate TRes Func<TSrc1, TSrc2, TSrc3, TRes>(TSrc1 src1, TSrc2 src2, TSrc3 src3);
public delegate TRes Func<TSrc1, TSrc2, TSrc3, TSrc4, TRes>(TSrc1 src1, TSrc2 src2, TSrc3 src3, TSrc4 s

Still, this could be cleaner.

Random Factories with Lambdas - The 3.5 Way

As Eric Lippert says:

"At first glance, lambda methods look like nothing more than a syntactic sugar, a more compact and pleasant syntax for embedding an anonymous method in code."

There are some subtle differences, and if you want to learn more I encourage you to check out Part 1, Part 2, Part 3, Part 4 and Part 5 of his detailed series on just those differences. For what we're doing, all that matters is that lambdas are nicer to look at.

Now, replacing the delegates with lambdas changes the delegate way:

List<FigureFactory<Brush, Figure>> foo = new List<FigureFactory<Brush, Figure>>();
foo.Add(delegate(Brush x) { return new SquareFigure(x); }); //etc...

to

var foo = new List
{
x => new SquareFigure(x), //etc...
}

So the whole random generator cleans up to just this chunk of code, with the list of figures moved out to a private field:

private static readonly List<Func<Brush, Figure>> listOfPotentialFigures = 
new List<Func<Brush, Figure>>
{
x => new SquareFigure(x),
x => new CircleFigure(x),
x => new TriangleFigure(x),
x => new StarFigure(x),
x => new HeartFigure(x),
x => new TrapezoidFigure(x),
x => new RectangleFigure(x)
};


private static Figure GenerateFigure(string letter)
{
var fill = Utils.GetRandomColoredBrush();
if (letter.Length == 1 && Char.IsLetterOrDigit(letter[0]))
return new LetterFigure(fill, letter);
var myFunc = listOfPotentialFigures[
Utils.RandomBetweenTwoNumbers(0, listOfPotentialFigures.Count-1)];
return myFunc(fill);
}

This feels MUCH cleaner than switch statement and it's easy to extend with a single line and it exactly expresses what I intend to do. In the future I may look at using an IOC (Inversion of Control) container to add figures dynamically via plugin or discovery model.

I'm interested in your comments as there's very likely an even cleaner pattern to solve this little problem in your head, waiting to get out.

Technorati Tags: ,,

About Scott

Scott Hanselman is a former professor, former Chief Architect in finance, now speaker, consultant, father, diabetic, and Microsoft employee. He is a failed stand-up comic, a cornrower, and a book author.

facebook twitter subscribe
About   Newsletter
Sponsored By
Hosting By
Dedicated Windows Server Hosting by ORCS Web
Friday, June 13, 2008 12:09:22 PM UTC
That's much cleaner!

Could you update the code on codeplex? I think I figured out a way to get rid of those magic numbers.
Friday, June 13, 2008 12:28:48 PM UTC
I know you moved on from the .NET 2 generics factory, and while I don't think reliance on a ctor param is a great way to factory..

You can however do activation with parameters.. the new() is really just rerouted to a call to System.Activator.CreateInstance<T>

This in turn is a call to System.Activator.CreateInstance(typeof(T), true)

This is saying, create a new instance of the type, looking for default parameterless ctors that are public.. you can however pass in parameters as an array.. something like

System.Activator.CreateInstance(typeof(T), new[] { fill });

Like I said though, reliance on a ctor with that signature isn't always a great idea.. you may instead go for an initialization interface..

interface IFigure
{
void Initialize(Fill fill);
}

var figure = new TFigure();
figure.Initialize(fill);

Just my 2p ;)
Stephen Taylor
Friday, June 13, 2008 1:16:30 PM UTC
>This feels MUCH cleaner than switch statement
Isn't there a trick of replacing the concrete selection logic with the random generator? :)
Anyway - great idea! And really nice step-by-step optimization, - what MSDN is usually missing. I hope that you guys will create good MSDN examples for MVC articles.
Friday, June 13, 2008 1:53:21 PM UTC
Hello,

I was recently involved in a wpf project, and was very disappointed. We were trying to do some client data processing and passing the data to a web service running on the server, it was discouraging to see that although wpf can call webservices, its not able to call secure web services which require user name authentication, for doing that a app has to run in full trust, which defeats the whole purpose.
in my view we should be able to call secure web services from our wpf apps

Ashish
Friday, June 13, 2008 2:32:12 PM UTC
Personally I find the below clearer. I thought returning midway through the body was frowned upon, but maybe that's just me.

private static Figure GenerateFigure(string letter)  
{
var fill = Utils.GetRandomColoredBrush();
Figure fig;

if (letter.Length == 1 && Char.IsLetterOrDigit(letter[0]))
fig = new LetterFigure(fill, letter);
else
fig = listOfPotentialFigures[
Utils.RandomBetweenTwoNumbers(0, listOfPotentialFigures.Count-1)](fill);

return fig;
}
Friday, June 13, 2008 3:04:40 PM UTC
Scott, I think you may have a typo in your delegates example. Line 4 of the second bit of code says FigureFactory<brush ,figure="">. I'm not sure what that figure="" is doing in the generic arguments.... If this is some valid syntax I'm just not familiar with, please shine some light!
Friday, June 13, 2008 4:51:03 PM UTC
This reminds me of Oren's code that he posted last week
Friday, June 13, 2008 5:34:26 PM UTC
Hi Scott,

Have you thought of using attributed classes for your object factory?

It would still be "extensible" because the class factory would only need to know about the assembly where to look instead of allowing "plug-ins" to access your internal list of objects.

HTH
DM
Friday, June 13, 2008 6:02:46 PM UTC
Scott Muc - Exactly! That's fantastic. Leave it to Oren to get it right in one try! I need to read his blog more and save myself some time. ;)

DM - Interesting idea, particularly for a plugin model. You have any good examples?

TurbulentIntellect - I'm not seeing this typo...what browser are you on?
Friday, June 13, 2008 7:43:29 PM UTC
MaxC - returning midway through a procedure is no longer frowned upon - that's a relic of old programming paradigms that doesn't apply with the shorter methods you (should) have with object oriented code.
Its is called using a Guard Clause, and can do wonders for the readability of code that has a bunch of special cases.
http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html
Friday, June 13, 2008 8:18:58 PM UTC
Hi Scott,

I first read about it on MSDN Mag March 2003... I recently needed this kind of pattern so I went back and found it here: http://msdn.microsoft.com/en-us/magazine/cc164170.aspx

HTH
DM
Friday, June 13, 2008 11:13:07 PM UTC
Curse you Scott Hanselman. You're making me think...
gary hanson
Saturday, June 14, 2008 2:39:59 PM UTC
Have you considered moving your private field definition out to a seperate file, as part of a partial class.

This way, when it comes to change management and source control, when a new figure comes along, you are not changing the class file that contains the main logic.
Sunday, June 15, 2008 12:24:43 PM UTC
Maybe we can discuss the pollution of the C# language at the Norwegian Developer Conference on tuesday? :)

Here's my version, no generics, no lambdas, no anonymous methods, no reflection... just a couple of objects:

class Shapes
{
static List<Shape> shapes = new List<Shape> {
new Square(), new Circle(), new Triangle(), new Star(), new Heart(), new Trapezoid(), new Rectangle() };

public static Shape GetRandom() {
return shapes[RandomBetweenTwoNumbers(0, shapes.Count - 1)];
}
}

class Program {
private static Figure GenerateFigure(char letter) {
Brush fill = Brush.GetRandom();
if (Char.IsLetterOrDigit(letter))
return new Figure(new Letter(letter), fill);
return new Figure(Shapes.GetRandom(), fill);
}
}

Sunday, June 15, 2008 6:02:19 PM UTC
Nice job Mashi! Yes, the issue with this problem is the Shape constructor's parameter. A parameterless constructor makes this easy as you've pointed out! How would you do your solution with a more complex constructor?
Monday, June 16, 2008 8:45:18 PM UTC
Great work on a cool program, Scott. I can see my son playing with it when he gets a little older. He's 9 months right now.

I'm wondering if showing random shapes it the best thing Baby Smash could do though. It seems like it might be better to cycle through a few pre-defined images for each key. That way, baby can eventually figure out that each key has meaning and different ones do different things. e.g. C - show a car, then a cat, then a cow, then the letter "C". The effect would still seem random when a bunch of keys are just smashed, but there would be more there for baby to figure out.
Tuesday, June 17, 2008 1:29:06 AM UTC
Not necessarily related to the post content, but you could simplify the Utils class a bit. There's no need fo rthe Dictionary of brushes just to print out the brush name. Enum has a method GetName that does that for you. Here are the relevant changes:

namespace BabySmash
{
class Utils
{
private static readonly Brush[] someBrushes = {
Brushes.Red,
Brushes.Blue,
Brushes.Yellow,
Brushes.Green,
Brushes.Purple,
Brushes.Pink,
Brushes.Orange,
Brushes.Tan,
Brushes.Gray,
};

public static string BrushToString(Brush b)
{
return Enum.GetName(typeof(Brush), b);
}
}
Tuesday, June 17, 2008 2:19:35 AM UTC
[sigh] Err, scratch that. Of course, Brush is not an enum. I need to brush on my WPF a bit...
Tuesday, June 17, 2008 6:38:41 AM UTC
I think you need to differentiate where you need to add static and runtime information. When you create a shape you have different object creation stages

a) Determine the shape types via configuration
b) Create an instance of the type
c) Set runtime info (random brush)

To enable that you need to add to the Figure class only
public Brush Fill
{
set;
get;
}

So your code would look something like this:

private static List<Type> figureTypes;

private static void ConfigureFigureTypesFromAppConfig()
{
var types = from string key in ConfigurationManager.AppSettings.Keys
where key.StartsWith("Figure")
let figureVale = ConfigurationManager.AppSettings[key]
select Type.GetType(figureVale);
figureTypes = types.ToList();
}

private static Figure GenerateFigure(string letter)
{
var fill = Utils.GetRandomColoredBrush();
if (letter.Length == 1 && Char.IsLetterOrDigit(letter[0]))
return new LetterFigure(fill, letter);


var randomFigureType = figureTypes[new Random().Next(0, figureTypes.Count-1)];
Figure randomFigure = (Figure) Activator.CreateInstance(randomFigureType);
randomFigure.Fill = fill;

return randomFigure;
}

And your shape list goes to the App.config file:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<appSettings>
<add key="Figure1" value="BabySmash.SquareFigure"/>
<add key="Figure2" value ="BabySmash.CircleFigure"/>
<add key="Figure3" value ="BabySmash.TriangleFigure"/>
<add key="Figure4" value ="BabySmash.StarFigure"/>
<add key="Figure5" value ="BabySmash.HeartFigure"/>
<add key="Figure6" value ="BabySmash.TrapezoidFigure"/>
<add key="Figure7" value ="BabySmash.RectangleFigure"/>
</appSettings>
</configuration>

Yours,
Alois Kraus

Comments are closed.

Disclaimer: The opinions expressed herein are my own personal opinions and do not represent my employer's view in any way.