4

I am developing an app that compares files. I decided to use the Strategy design pattern, to handle different formats, so I have something like this:

public class Report {
   CompareStrategy strategy;
   ...
}


public interface CompareStrategy {
   int compare(InputStream A, InputStreamB);
}

Then, naturally I implement the compare method for different file formats.

Now suppose I wanted to add another method, that deals with certain restrictions for the comparison (e.g. omit a row in case of an Excel or csv file, or omit a node in XML).

Would it be better to:

  1. Add another method to the interface and every implementation (there are few for the moment)
  2. Write a new interface which inherits from CompareStrategy and then implement it?

The second question is: since the differences can be of various types - would it be OK to make a marker interface Difference to enable something like:

int compareWithDifferences(..., Iterable<Difference> differences);

and then go on defining what a difference means for the specific file format?

DCzo
  • 141
  • 2
  • 14
  • Your design seems to violate the LSP because depending on what the input stream contains different strategies would succeed or fail and that would be unexpected because swapping one strategy for another shouldn't affect the correctness of the program. You should really question whether the interface brings you value or not. If it does it should at least be made explicit that a strategy can't handle any kind of content e.g. `bool canCompare(a, b)` – plalx Aug 01 '17 at 07:40
  • Could you elaborate? The strategies I designed are tightly connected with the format of the file (some are table-like other are in markup languages). The use of InputStream is meant to facilitate unit testing. – DCzo Aug 01 '17 at 08:49
  • What I mean is that you must know which instances can compare which file types which defeats the purpose of the interface in the first place because the application must have knowledge of the implementations and can't just work only with `CompareStrategy` without caring about which implementation is provided. If you need extensibility then you most likely need such an interface so that you can register new comparators dynamically, but that's not the strategy pattern and it must be clear from the interface that calling a comparator with the wrong content might throw. – plalx Aug 01 '17 at 13:18
  • But isn't it true, that the Strategy pattern defines a different strategy for different context? That is exactly this case, as a method for parsing a file is not detachable from the file format. I am following the principles described here: https://sourcemaking.com/design_patterns/strategy – DCzo Aug 01 '17 at 13:39
  • From your very own source... "A Strategy defines a set of algorithms that can be used interchangeably". That's not your case at all, you can't interchangeably use the `XMLCompareStrategy` with HTML content for instance. Therefore `strategy1.compare(a, b)` might work while `strategy2.compare(a, b)` might fail for the same `a` and `b`. That shouldn't be the case with the strategy pattern. – plalx Aug 01 '17 at 13:47
  • You are right, yet I would still like to have some neat way of applying the comparison without using a bloated switch instruction (there will probably be more file formats than I am dealing with now and the app will compare lots of reports). The interface allows me to call something like: `report.getStrategy.compare(a, b)` without worrying too much about the format. What other pattern would you recommend? – DCzo Aug 01 '17 at 13:59
  • Not necessarily another pattern, but I'm just saying it's not an implementation of the strategy pattern. I'd probably just go for a `Comparator` interface and add a throws declaration in the interface to make it explicit that it might fail. – plalx Aug 01 '17 at 14:33
  • Do you mean any existing exception or should I write a custom one? – DCzo Aug 01 '17 at 14:51
  • Probably a custom one, unless there is an existing one that conveys the meaning. – plalx Aug 01 '17 at 19:51
  • I will try this approach, thank you! – DCzo Aug 01 '17 at 20:50

3 Answers3

4

Now suppose I wanted to add another method, that deals with certain restrictions for the comparison (e.g. omit a row in case of an Excel or csv file, or omit a node in XML).

Looks like you need the Template Pattern

You can create some abstract class like

public abstract class XMLCompareStrategy implements CompareStrategy {

    public int compare(InputStream A, InputStreamB) {
        // several steps
        customMethod(...);
        // more steps
    }

    protected abstract ... customMethod(...);

}

This way you can create several classes that have the main or core functionality and provide custom details for every situation

Alberto S.
  • 7,409
  • 6
  • 27
  • 46
  • Nice one. Short and precise! – GhostCat Aug 01 '17 at 07:18
  • So (just to satisfy my classification mania ;)) - this would be a combination of Strategy (base interface implemented to abstract classes for each format) with Template (abstract classes extended to concrete classes according to their functionality). Is that correct? – DCzo Aug 01 '17 at 07:32
  • Yep. With Strategy you decouple the actual implementation for each format and with Template you provide a base implementation for each format and also gives you with several points of algorithm customization – Alberto S. Aug 01 '17 at 07:40
  • 1. An abstract class is a good solution if the `compare` implementation is common to all the implementing classes. 2. Keep in mind that by using an abstract class instead of an interface, you lose the ability to extend any other class. – SHG Aug 01 '17 at 07:47
  • If extending a class becomes a trouble you can always get rid of abstract methods by implementing again an Strategy pattern ;) – Alberto S. Aug 01 '17 at 08:01
2

The answer really depends on your needs:

  • If the method will always be implemented - add it to the existing interface. If it's only optional - create a new interface that will extend the current one, and then the implemented class could implement either the base interface or the child interface if it needs both methods.
  • For your second question - looks a bit like over-designing to me, but again depends on your needs.
SHG
  • 2,516
  • 1
  • 14
  • 20
0

I think you should maybe write another interface that inherit from the CompareStrategy. Like that if you need to compareWithDifferences() you can, but you don't have to use this interface you still can use the simpler one with no differences.

As Jonathan said, if you can foresee difficulties prepare for it. In that case I think you should prepare. Indeed that won't cost you much time to create another interface and you won't have to refactor later.

GhostCat
  • 137,827
  • 25
  • 176
  • 248
Alexandre Thyvador
  • 364
  • 1
  • 10