4

For example, consider a utility class SerializableList:

public class SerializableList : List<ISerializable>
{
    public T Add<T>(T item) where T : ISerializable
    {
        base.Add(item);
        return item;
    }

    public T Add<T>(Func<T> factory) where T : ISerializable
    {
        var item = factory();
        base.Add(item);
        return item;
    }
}

Usually I'd use it like this:

var serializableList = new SerializableList(); 
var item1 = serializableList.Add(new Class1());
var item2 = serializableList.Add(new Class2());

I could also have used it via factoring, like this:

var serializableList = new SerializableList(); 
var item1 = serializableList.Add(() => new Class1());
var item2 = serializableList.Add(() => new Class2());

The second approach appears to be a preferred usage pattern, as I've been lately noticing on SO. Is it really so (and why, if yes) or is it just a matter of taste?

noseratio
  • 59,932
  • 34
  • 208
  • 486
  • The second approach use lambda expression syntax, and I don't think there's any differences between those 2 approachs – Doan Cuong Oct 09 '13 at 03:48
  • I don't see the point of passing of the invocation to the list. – tuespetre Oct 09 '13 at 03:54
  • @DoanCuong, I knew the word `lambda` would pop up, so I've updated the title to endorse it :) – noseratio Oct 09 '13 at 03:56
  • "The second approach appears to be a preferred usage pattern..." Where? It seems to be pointless and inefficient. Could be there's a use case in the other examples that I'm missing? – NPSF3000 Oct 09 '13 at 04:03
  • @NPSF3000, [here](http://stackoverflow.com/a/19218822/1768303) and [here](http://stackoverflow.com/a/19239509/1768303), I could try finding more. My understanding is, perhaps, because the 2nd approach creates an object in the context of the consuming method (`Add`), it leaves an option to handle any exceptions inside that method. Although that's not what happens in those code fragments. Also, I believe C# compiler is smart enough to optimize such code, so both approaches are equally efficient, at least for my simple example. – noseratio Oct 09 '13 at 04:13
  • @Noseratio - if you're using it to handle exceptions in a specific way (and make this very clear) then I could understand it. Beyond that though it's less clear what's going on - for example we no longer know *when* that object is being created (albeit helped somewhat by the return) or how many times that object will be created (could be 0 if an exception occurs, could be more if the code does something you don't expect). – NPSF3000 Oct 09 '13 at 04:18
  • @NPSF3000, My personal preference is the 1st approach. With the 2nd one, I'm not using it to handle any exceptions inside `Add`, but so are not the authors of those code fragments (if I understand their code correctly). Yet, I believe both authors are more experienced with C# than I am, that's why I'm raising this question. – noseratio Oct 09 '13 at 04:25
  • @Noseratio As I understand the references in your comment, this specific approach is used to handle some issues with `IDisposable`s. This is not a general approach. – FrankPl Oct 11 '13 at 15:18
  • @FrankPl, those references do not catch any exceptions in the scope where the factoring lambda is used. The `new Class` syntax could as well have been used there, if I'm not mistaken. – noseratio Oct 11 '13 at 23:51

4 Answers4

3

Given your example, the factory method is silly. Unless the callee requires the ability to control the point of instantiation, instantiate multiple instances, or lazy evaluation, it's just useless overhead.

The compiler will not be able to optimize out delegate creation.

To reference the examples of using the factory syntax that you gave in comments on the question. Both examples are trying (albeit poorly) to provide guaranteed cleanup of the instances.

If you consider a using statement:

using (var x = new Something()) { }

The naive implementation would be:

var x = new Something();
try 
{ 
}
finally
{
   if ((x != null) && (x is IDisposable))
     ((IDisposable)x).Dispose();
}

The problem with this code is that it is possible for an exception to occur after the assignment of x, but before the try block is entered. If this happens, x will not be properly disposed, because the finally block will not execute. To deal with this, the code for a using statement will actually be something more like:

Something x = null;
try 
{
   x = new Something();
}
finally
{
   if ((x != null) && (x is IDisposable))
      ((IDisposable)x).Dispose();
}

Both of the examples that you reference using factory parameters are attempting to deal with this same issue. Passing a factory allows for the instance to be instantiated within the guarded block. Passing the instance directly allows for the possibility of something to go wrong along the way and not have Dispose() called.

In those cases, passing the factory parameter makes sense.

William
  • 1,867
  • 11
  • 10
  • I agree with this, although I feel I still may be missing something. BTW, why do you think the compiler doesn't optimize (or inline) simple lambdas like these, have you checked the IL code of a Release build (I haven't)? – noseratio Oct 14 '13 at 03:50
  • 1
    I haven't checked a release build specifically, but I did verify in a debug build. The compiler can't optimize away the delegate, because the callee method expects a delegate. You're not missing anything, each technique serves a purpose in the right scenario. Passing in a factory is just a very specialized case. – William Oct 14 '13 at 05:06
  • Regarding the "naive" implementation of `using` in your sample, isn't it exactly how compiler implements it: http://msdn.microsoft.com/en-us/library/yh598w02.aspx ? The `new` statement is placed outside the `try` block. – noseratio Oct 15 '13 at 04:19
  • Yes, it is. I thought it was changed in the past, not that it would matter because it would still be possible to not call `Dispose` in cases of well timed asynchronous exceptions. My mistake, but I don't feel that it invalidates my general concept. – William Oct 15 '13 at 04:41
  • Right, the concept remains valid, thanks for your efforts, +1 already. – noseratio Oct 15 '13 at 04:56
  • You were right about optimizations for this on the IL code level, there are none. It'd be interesting to know if IL-to-native JIT compiler takes this any further. – noseratio Oct 17 '13 at 05:41
2

Caching

In the example you have provided it does not make sense as others have pointed out. Instead I will give you another example,

public class MyClass{
    public MyClass(string file){
        // load a huge file
        // do lots of computing...
        // then store results...
    }
}

private ConcurrentDictionary<string,MyClass> Cache = new ....

public MyClass GetCachedItem(string key){
    return Cache.GetOrAdd(key, k => new MyClass(key));
}

In above example, let's say we are loading a big file and we are calculating something and we are interested in end result of that calculation. To speedup my access, when I try to load files through Cache, Cache will return me cached entry if it has it, only when cache does not find the item, it will call the Factory method, and create new instance of MyClass.

So you are reading files many times, but you are only creating instance of class that holds data just once. This pattern is only useful for caching purpose.

But if you are not caching, and every iteration requires to call new operator, then it makes no sense to use factory pattern at all.

Alternate Error Object or Error Logging

For some reason, if creation fails, List can create an error object, for example,

 T defaultObject = ....

public T Add<T>(Func<T> factory) where T : ISerializable
{
    T item;
    try{
        item = factory();
    }catch(ex){
        Log(ex);
        item = defaultObject;
    }
    base.Add(item);
    return item;
}

In this example, you can monitor factory if it generates an exception while creating new object, and when that happens, you Log the error, and return something else and keep some default value in list. I don't know what will be practical use of this, but Error Logging sounds better candidate here.

Akash Kava
  • 39,066
  • 20
  • 121
  • 167
1

No, there's no general preference of passing the factory instead of the value. However, in very particular situations, you will prefer to pass the factory method instead of the value.

Think about it:

What's the difference between passing the parameter as a value, or passing it as a factory method (e.g. using Func<T>)?

Answer is simple: order of execution.

  • In the first case, you need to pass the value, so you must obtain it before calling the target method.
  • In the second case, you can postpone the value creation/calculation/obtaining till it's needed by the target method.

Why would you want to postpone the value creation/calculation/obtaining? obvious things come to mind:

  • Processor-intensive or memory-intensive creation of the value, that you want to happen only in case the value is really needed (on-demand). This is Lazy loading then.
  • If the value creation depends on parameters that are accessible by the target method but not from outside of it. So, you would pass Func<T, T> instead of Func<T>.
Tengiz
  • 8,011
  • 30
  • 39
1

The question compares methods with different purposes. The second one should be named CreateAndAdd<T>(Func<T> factory).

So depending what functionality is required, should be used one or another method.

Michael Freidgeim
  • 26,542
  • 16
  • 152
  • 170