Pergunta

In some of my code, I have a static factory similar to this:

public class SomeFactory {

    // Static class
    private SomeFactory() {...}

    public static Foo createFoo() {...}

    public static Foo createFooerFoo() {...}
}

During a code review, it was proposed that this should be a singleton and injected. So, it should look like this:

public class SomeFactory {

    public SomeFactory() {}

    public Foo createFoo() {...}

    public Foo createFooerFoo() {...}
}

A few things to highlight:

  • Both factories are stateless.
  • The only difference between the methods are their scopes (instance vs static). The implementations are the same.
  • Foo is a bean that does not have an interface.

The arguments I had for going static was:

  • The class is stateless, therefore doesn't need to be instantiated
  • It seems more natural to be able to call a static method than to have to instantiate a factory

The arguments for the factory as a singleton was:

  • Its good to inject everything
  • Despite the statelessness of the factory, testing is easier with injection (easy to mock)
  • It should be mocked when testing the consumer

I have some serious issues with the singleton approach since it seems to suggest that no methods should ever be static. It also seems to suggest that utilities such as StringUtils should be wrapped and injected, which seems silly. Lastly, it implies that I'll need to mock the factory at some point, which doesn't seem right. I can't think of when I'd need to mock the factory.

What does the community think? While I don't like the singleton approach, I don't seem to have a terribly strong argument against it.

Foi útil?

Solução

Why would you separate your factory from the object-type it creates?

public class Foo {

    // Private constructor
    private Foo(...) {...}

    public static Foo of(...) { return new Foo(...); }
}

This is what Joshua Bloch describes as his Item 1 on page 5 of his book, "Effective Java." Java is verbose enough without adding extra classes and singletons and whatnot. There are some instances where a separate factory class makes sense, but they are few and far between.

To paraphrase Joshua Bloch's Item 1, unlike constructors, static factory methods can:

  • Have names that can describe specific kinds of object creation (Bloch uses probablePrime(int, int, Random) as an example)
  • Return an existing object (think: flyweight)
  • Return a sub-type of the original class or an interface it implements
  • Reduce verbosity (of specifying type parameters - see Bloch for example)

Disadvantages:

  • Classes without a public or protected constructor cannot be subclassed (but you can use protected constructors and/or Item 16: favor composition over inheritence)
  • Static factory methods do not stand out from other static methods (use a standard name like of() or valueOf())

Really, you should take Bloch's book to your code reviewer and see if the two of you can agree to Bloch's style.

Outras dicas

Just off the top of my head, here are some of the problems with statics in Java:

  • static methods don't play by the OOP "rules". You can't force a class to implement specific static methods (as far as I know). So you can't have multiple classes implementing the same set of static methods with the same signatures. So you're stuck with one of whatever it is you're doing. You can't really subclass a static class, either. So no polymorphism, etc.

  • since methods aren't objects, static methods can't be passed around and they can't be used (without doing a ton of boilerplate coding), except by code that is hard-linked to them -- which makes the client code highly coupled. (To be fair, this isn't solely the fault of statics).

  • statics fields are pretty similar to globals


You mentioned:

I have some serious issues with the singleton approach since it seems to suggest that no methods should ever be static. It also seems to suggest that utilities such as StringUtils should be wrapped and injected, which seems silly. Lastly, it implies that I'll need to mock the factory at some point, which doesn't seem right. I can't think of when I'd need to mock the factory.

Which makes me curious to ask you:

  • what, in your opinion, are the advantages of static methods?
  • do you believe that StringUtils is correctly designed and implemented?
  • why do you think you'll never need to mock the factory?

I think the Application you're building must be fairly uncomplicated, or else you're relatively early in its lifecycle. The only other scenario I can think of where you wouldn't have already run into problems with your statics is if you've managed to limit the other parts of your code that know about your Factory to one or two places.

Imagine that your Application evolves and now in some instances you need to produce a FooBar (subclass of Foo). Now you need to go through all places in your code that know about your SomeFactory and write some kind of logic that looks at someCondition and calls SomeFactory or SomeOtherFactory. Actually, looking at your example code, it's worse than that. You plan on just adding method after method for creating different Foos and all your client code has to check a condition before figuring out which Foo to make. Which means all clients need to have an intimate knowledge of how your Factory works and they all need to change whenever a new Foo is needed (or removed!).

Now imagine if you just created an IFooFactory that makes an IFoo. Now, producing a different subclass or even a completely different implementation is child's play--you just feed in the right IFooFactory higher up the chain and then when the client gets it, it calls gimmeAFoo() and will get the right foo because it got the right Factory.

You might be wondering how this plays out in production code. To give you one example from the codebase I'm working on that's starting to level out after just over a year of cranking out projects, I have a bunch of Factories that are used to create Question data objects (they're something like Presentation Models). I have a QuestionFactoryMap that's basically a hash for looking up which question factory to use when. So to create an Application that asks different questions, I put different Factories into the map.

Questions can be presented in either Instructions or Exercises (which both implement IContentBlock), so each InstructionsFactory or ExerciseFactory will get a copy of the QuestionFactoryMap. Then, the various Instructions and Exercise factories are handed to the ContentBlockBuilder which again maintains a hash of which to use when. And then when the XML is loaded in, the ContentBlockBuilder calls up the Exercise or Instructions Factory out of the hash and asks for it to createContent(), and then the Factory delegates the building of the questions to whatever it pulls out of its internal QuestionFactoryMap based on what is in each XML node vs. the hash. Unfortunately, that thing is a BaseQuestion instead of an IQuestion, but that just goes to show that even when you're being careful about design you can make mistakes that can haunt you down the line.

Your gut is telling you that these statics will hurt you, and there's certainly plenty out there in the literature to support your gut. You will never lose by having Dependency Injection that you wind up only using for your Unit Tests (not that I believe that if you build good code that you won't find yourself using it more than that), but statics can completely destroy your ability to quickly and easily modify your code. So why even go there?

Licenciado em: CC-BY-SA com atribuição
scroll top