Mapping factory methods with anonymous functions/delegates using Dictionary for faster lookup?

4.1k views Asked by At

Currently, I have a static factory method like this:

public static Book Create(BookCode code) {
    if (code == BookCode.Harry) return new Book(BookResource.Harry);
    if (code == BookCode.Julian) return new Book(BookResource.Julian);
    // etc.
}

The reason I don't cache them in any way is because BookResource is sensitive to culture, which can change between the calls. The changes to culture needs to reflect in the returned book objects.

Doing those if-statements is possible a speed bottleneck. But what if we map book codes to anonymous functions/delegates? Something like the following:

delegate Book Create();
private static Dictionary<BookCode, Delegate> ctorsByCode = new Dictionary<BookCode, Delegate>();
// Set the following somewhere
// not working!
ctorsByCode[BookCode.Harry] = Create () => { return new Book(BookResource.Harry); }
// not working!
ctorsByCode[BookCode.Julian] = Create () => { return new Book(BookResource.Julian); } 
public static Book Create(BookCode code) {
    return (Book)ctorsByCode[code].DynamicInvoke(null);
}

How could I get those Create() => { lines to actually work?

Is this worth it speed-wise, when there are <50 book codes (thus <50 if-statements)?

This is a similar question, but unfortunately the author doesn't post his code Enum, Delegate Dictionary collection where delegate points to an overloaded method

Update

Did some performance benchmarks ifs vs delegates. I picked the unit code randomly and used the same seed for both methods. The delegate version is actually slightly slower. Those delegates are causing some kind of overhead. I used release builds for the runs.

5000000 iterations
CreateFromCodeIf ~ 9780ms
CreateFromCodeDelegate ~ 9990ms
3

There are 3 answers

3
SLaks On BEST ANSWER

Like this:

private static readonly Dictionary<BookCode, Func<Book>> ctorsByCode = new Dictionary<BookCode, Func<Book>>();

...

ctorsByCode[BookCode.Harry] = () => new Book(BookResource.Harry);

public static Book Create(BookCode code) {
    return ctorsByCode[code]();
}

The Func<Book> type is a pre-defined generic delegate type that you can use instead of creating your own.
Also, your dictionary must contain specific a delegate type, not the base Delegate class.

Although lambda expressions are untyped expressions, they can be used in a place where a delegate type is expected (such as your indexer assignment) and will implicitly assume the type of the delegate.

You could also write

ctorsByCode[BookCode.Harry] = new Func<Book>(() => new Book(BookResource.Harry));

I would recommend changing your dictionary to contain BookResources instead, like this:

private static readonly Dictionary<BookCode, BookResource> codeResources = new Dictionary<BookCode, BookResource>();

...

codeResources[BookCode.Harry] = BookResource.Harry;

public static Book Create(BookCode code) {
    return new Book(codeResources[code]);
}
1
epitka On

randomguy, why are you complicating things so much? Two ways to avoid this mess in your question and in the answers that followed.

  1. BookResource bookResourceEnum = (BookResource)Enum.Parse(typeof(BookResource),bookCode);

    return new Book(bookResourceEnum);

  2. Or better yet, make your static factory method Create accept BookResource instead of the BookCode, and let clients worry about the conversion from code to enum so that nobody can pass in a code that cannot be mapped to BookResource.

Seriously, if you wrote me a code with 50 "if" or worse yet with 50 "delegates" we would have a "talk". Try to avoid premature optimization (if there is any) when you are not sure that is your bottle neck. Readability and maintainability should come first.

1
Keltex On

Frankly you probably will not notice much a performance difference between the two. But if you're going to use the "functional" version, I would not just use a generic Delegate but instead pass the delegate that you just created. So this would be your (simpler) code:

delegate Book Create();
private static Dictionary<BookCode, Create> ctorsByCode 
    = new Dictionary<BookCode, Create>();

ctorsByCode[BookCode.Harry] = () => new Book(BookResource.Harry);
ctorsByCode[BookCode.Julian] = () => new Book(BookResource.Julian); 

public static Book Create(BookCode code) {
    return ctorsByCode[code]();
}