On global variables and factories...

On global variables and factories...

Imagine I have a factory class called TFactory, implementing the interface IFactory. Now, if I wish to use this factory, I need to instantiate it and bind it to a variable in some way or another.

There are several ways of doing this:
1) Bind it to a local variable, requiring an additional local variable in all functions using it.
2) Bind it to a global variable.
3) Bind it to a variable in the implementation part of a unit, and expose it by a function GetFactory: IFactory.
4) Make a function GetFactory: IFactory that constructs a TFactory and bind it to Result, thereby handling the destruction of the TFactory without needing to bind it to a local variable.

There are probably more ways to do this (such as using a singleton), but the point here is really just to avoid the need to destroy the factory manually, because that would be tedious

I'd be loathe to use either option 1 or 2. But there's no functional difference (that I can see) between using option 3 and option 4, but option 3 seems more "efficient", because we only need to create the object once.

I'd personally choose option 4, as it is threadsafe, and I don't think it would be measurable performance-wise.

What would you do, and why?

Comments

  1. Question: why are you using a factory behind an interface?

    My guess is: decoupling and putting the responsibility of creation into its own class which are good reasons. Now if we are talking about decoupling any of your presented ways are introducing another dependency that is worse than directly hardcoding constructor calls. I can't explain why any better than Misko Hevery does, that's why I just put a link here (yes, it is a long read but I recommend reading it carefully): http://misko.hevery.com/code-reviewers-guide/flaw-brittle-global-state-singletons/

    The best solution imo is to use DI and inject your IFactory where you need it.

    ReplyDelete
  2. Stefan Glienke Because then I can write

    function GetFactory: IFactory;
    begin
    Result := TFactory.Create;
    end

    o := GetFactory.MakeInstance;

    and not worry about memory management, and thus I don't have to bind it to a local variable.

    ReplyDelete
  3. Imo you are fooling yourself in the long run with such an approach. You are gaining absolutely nothing. You are creating a factory instance every time you are calling the GetFactory function. So obviously it has a state (maybe it creates even more objects in its constructor, who knows). But that state only exists for one moment because after using the factory it is destroyed again. Then you are coupling your code to the GetFactory function (hello service locator anti pattern). You could as well make a static MakeInstance function that returns whatever you want for o. It would not make any difference because in both cases you have hardwired your code.

    ReplyDelete
  4. So you'd rather make a class function of some sort, that can be called with TFactory.MakeInstance ?

    ReplyDelete
  5. No, I would inject the IFactory to the class that needs it (and yes, that shifts the location where it has to be created to another place). That way I have decoupled both pieces, the consumer of the factory and the implementation of it.

    ReplyDelete
  6. That makes sense, but where does it end? Who is responsible for creating "the first factory" or class?

    ReplyDelete
  7. I am kind of expecting a "bright, shining light" moment here.

    ReplyDelete
  8. Again I let someone explain who has way more experience and knowledge about DI than I do :) http://blog.ploeh.dk/2011/07/28/CompositionRoot/

    ReplyDelete
  9. Or you could just pass to your class TFunc instead of your TFactory. Then your class will be easier to test.

    ReplyDelete
  10. I like property-as-a-function

    TFooFactory = class
    private class var
    _FooBuilder: TFunc;
    public
    class property BuildFoo : TFunc read GetFooBuilder write _FooBuilder;
    end;

    // get Instance
    myFoo := TFooFactory.BuildFoo();

    // set factory
    TFooFactory.BuildFoo := function : IFoo
    begin
    Result := TSomeFoo.Create();
    end;

    ReplyDelete
  11. Oliver Münzberg You don't gain anything by doing this IMO. You just add more boilerplate. Also, you should try to avoid injecting dependencies through properties. Constructor injection should be preferred. To me this is much more readable and safe:

    TClassWithFooDependency = class
    private
    FooFactory: TFunc;
    public
    constructor Create(const fooFactory: TFunc);

    //get foo from factory
    var foo: IFoo;
    begin
    foo := FooFactory();

    ReplyDelete
  12. Stefan Glienke When looking at it from the point of a Delphi Forms application, would you say that the "root" would be the FormCreate event of your main form?

    ReplyDelete
  13. Kasper Brohus Allerslev No, the root is in the dpr file and the configuration should be done before creating any form

    ReplyDelete
  14. Since this thread turned out to be so great (for me at least), I'm going to keep it alive with related questions.

    Oliver Münzberg
    It seems that this would emphasize creating my entire application as a singular object, and let that object function as an API to be used in a command line / forms application.

    That seems like a really good idea. But how do I pass this object to my forms, without adding it to the global namespace?

    I have to inject the object somehow, and it seems unsafe to do so with property injection, but it isn't possible (as far as I'm aware) to inject it via constructors.

    ReplyDelete
  15. Well, a root points always to one point.

    It does not matter which DI strategy you choose, but you will always have a single point of composition (at the very first start of the application) and a single root (well one root should always be single).

    Someone (the root) has to know what to inject, when and where (constructor, field).

    So in the end you will have minimum one "singleton" (it does not matter if it is designed with the single pattern, it matters how you treat it).

    BTW Application: TApplication is also a "singleton". You can create more than one instance, but it would not make sense at all.

    ReplyDelete

Post a Comment