2

We have multiple publishers, that publish the data somewhere. For example, we have ElasticSearch publisher, AWS S3 publisher, or file publisher.

Now the interface looks like this

interface Publisher<T>{
  void publish(T data);
}

now ElasticSearch or AWS S3 publisher does not require another parameters, it can get all parameters it needs from config file. However the file publisher does need the parameter - the name of the file which is not stored in config like for previous publishers ( its actually jvm argument in script - legacy reason i guess)

I am trying to create a factory for publishers:

public class PublisherFactor{

    public Publisher create(String type){
          switch(type){
              case "aws":
                return new AwsPublisher();
              case "elastic"
                return new ElasticPublisher();
              case "file"
                return new FilePublisher(fileName);
          }
     }
}

What is the "pretty" way to use Factory pattern for this, without putting another parameter in create method that would be not used?

Can it be used like this? This is just small helper application. It is being run something like this:

javac <app> <publisher type> <if its file then file path>

The publisher type is decided in the script that invoke the command above. It determines the publisher according to avaibility.

Thanks for help!

3

Think of it like this - when you first created the application Publisher create(String type) was a factory, and the design was predicated on the assumption that a single string denoting the publisher type was enough to determine which concrete publisher to construct. As long as this assumption remained true, this was a good abstraction and this factory was nicely decoupled. However, now that assumption no longer holds and it's time to take a deeper look.

So, now, under these new circumstances, it becomes a bit more clear that this function, as originally envisioned, is going to be juggling more than one responsibility: besides being a factory, it needs to interpret the meaning of command-line input ("if type is file, a file path is also needed").

Note however that, since this is a small, "helper" command line tool, it may not be worth the effort too invest to much time into achieving a better design. It may not be worth separating these responsibilities out. An "inelegant" design that's easy to understand, and that works, is not necessarily going to be a problem, especially if you expect the code to be modified only very rarely. On the other hand, if you keep adding features, and your functions just keep growing and the code keeps getting more and more spaghetti-like, it's worth cleaning up the code - especially the parts that are the most susceptible to change.

So, you can do a couple of things here. First, you can reconceptualize your create method; instead of thinking of it as a factory, think of it as something that interprets the input. If that is its purpose, then it makes sense that some of the parameters will not be used, as that depends on how the command-line API is designed. Its job is now to encapsulate that logic. You can also change the name to communicate the idea, if you like:

Publisher interpretPublisherArgs(String type, String fileName)

// or perhaps

Publisher interpretPublisherArgs(PublisherArgs args)

You can go a step further, though. If it's more convenient to return a factory object (just a lambda really), rather then the publisher itself, and pass this factory to some other code, you can do something like this:

public Supplier<Publisher> interpretPublisherArgs(String type, String fileName) {
      switch(type){
          case "aws":
            return () -> new AwsPublisher();
          case "elastic":
            return () -> new ElasticPublisher();
          case "file":
            // check if 'fileName' is valid
            return () -> new FilePublisher(fileName);
      }
}

This decouples argument interpretation time from publisher construction time. In some circumstances, that's useful; this may or may not be the case for your code, so I don't recommend doing this by default - see if you'll actually benefit from this first. Here, the factory takes no arguments, but in some scenarios, it might be the case that an extra argument is required to construct each of the objects, but the argument only becomes available at a later point in time (e.g., after a user is prompted for some input). Returning a factory that takes an argument lets you "remember" a part of the creation process, and complete it later, when the required input becomes available.

If you go for this design, then, after you obtain command line arguments, you can do:

String type = args[0];
String fileName = args.length > 1 ? args[1] : "";

Supplier<Publisher> publisherFactory = interpretPublisherArgs(type, fileName);
// pass publisherFactory along...

// ...

// somewhere else:
Publisher publisher = publisherFactory.get();
3
  • I'd change interpretPublisherArgs(type, fileName) to getPublisherFactory(args) or getPublisherSupplier(args) since that's what the function does. Jun 22 at 16:26
  • @TulainsCórdova that's fine too, but that's a matter of one's coding/communication style. IMO, while it's true that a name like that quite directly describes a part of what the function does, it doesn't communicate the other significant thing that the function does, which I wanted to emphasize as more important (and more directly related to its primary concern), and it's also somewhat superfluous since the same information can be seen from the return type. In this case I made a judgement call that going for a "nonstandard" name was worth it, prioritizing "what it's for" over "what it does". Jun 22 at 17:05
  • I prefer this solution because if you up and decide the filename should be an overridable default value all you have to do is type it where the empty string is. If you decide the filename should come from a properties file that's also not mixed up in the publisherFactory.get(). You can decided how to find the file name at a low level and leave the rest of the code blissfully ignorant of how it was found. Jun 22 at 20:29
2

It is usually the responsibility of the Factory object to contain any additional parameters required for the constructors, which are not already in requested by the create method signature.

In your case, this additional parameter would be the fileName, and you're right that this would not be so nice, because it would not always be used for all instances being created and it would be a "broken window", where more and more fields could be added to the factory in the future. However, you can avoid this issue by using a more general Properties or Context object, so the Factory can pass the required values forward to the created instances without knowing any details about them.

E.g., you can instantiate a simple Properties (docs here) containing key-value pairs for any command-line inputs or default values, and then pass that to the PublisherFactory. Then, in the create method, you can either pass the Properties forward to FilePublisher, or directly get the fileName from it and pass only that. An advantage of passing the entire Properties object, is that additional command-line parameters will not require further changes to the Factory.

1
  • And you might want to have some error checking that detects if properties go unused.
    – Erik Eidt
    Jun 22 at 14:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.