29

I'm going to save some string payload in the database. I have two global configurations:

  • encryption
  • compression

These can be enabled or disabled using the configuration in a way that either only one of them is enabled, both are enabled or both are disabled.

My current implementation is this:

if (encryptionEnable && !compressEnable) {
    encrypt(data);
} else if (!encryptionEnable && compressEnable) {
    compress(data);
} else if (encryptionEnable && compressEnable) {
    encrypt(compress(data));
} else {
  data;
}

I'm thinking about the Decorator pattern. Is it the right choice, or is there perhaps a better alternative?

Nick Alexeev
  • 2,532

6 Answers6

117

The only problem I see with your current code is the risk of combinatorial explosion as you add more settings, which can be easily be mitigated by structuring the code more like this:

if(compressEnable){
  data = compress(data);
}
if(encryptionEnable) {
  data = encrypt(data);
}
return data;

I'm not aware of any "design pattern" or "idiom" that this could be considered an example of.

Ixrec
  • 27,711
18

When designing code, you alaways have two options.

  1. just get it done, in which case pretty much any solution will work for you
  2. be pedantic and design a solution which exploits the quirks of the language an its ideology (OO languages in this case - the use of polymorphism as a mean to provide the decision)

I am not going to focus on the first of the two, because there is really nothing to be said. If you just wanted to get it to work, you could leave the code as it is.

But what would happen, if you chose to do it the pedantic way and actually solved the problem with design patterns, in the way you wanted it do?

You could be looking at the following process:

When designing OO code, most of the ifs which are in a code do not have to be there. Naturally, if you want to compare two scalar types, such as ints or floats, you are likely to have an if, but if you want to change procedures based on configuration, you can use polymorphism to achieve what you want, move the decisions (the ifs) from your business logic to a place, where objects are instantiated - to factories.

As of now, your process can go through 4 separate paths:

  1. data is neither encrypted nor compressed (call nothing, return data)
  2. data is compressed (call compress(data) and return it)
  3. data is encrypted (call encrypt(data) and return it)
  4. data is compressed and encrypted (call encrypt(compress(data)) and return it)

Just by looking at the 4 paths, you find a problem.

You have one process which calls 3 (theoretically 4, if you count not calling anything as one) different methods that manipulate the data and then returns it. The methods have different names, different so called public API (the way through which the methods communicate their behaviour).

Using the adapter pattern, we can solve the name colision (we can unite the public API) that has occured. Simply said, adapter helps two incompatible interfaces work together. Also, adapter works by defining a new adapter interface, which classes trying to unite their API implement.

This is not a concrete language. It is a generic approach, the any keyword is there to represent it may be of any type, in a language like C# you can replace it with generics (<T>).

I am going to assume, that right now you can have two classes responsible for compression and encryption.

class Compression
{
    Compress(data : any) : any { ... }
}

class Encryption
{
    Encrypt(data : any) : any { ... }
}

In an enterprise world, even these specific classes are very likely to be replaced by interfaces, such as the class keyword would be replaced with interface (should you be dealing with languages like C#, Java and/or PHP) or the class keyword would stay, but the Compress and Encrypt methods would be defined as a pure virtual, should you code in C++.

To make an adapter, we define a common interface.

interface DataProcessing
{
    Process(data : any) : any;
}

Then we have to provide implementations of the interface to make it useful.

// when neither encryption nor compression is enabled
class DoNothingAdapter : DataProcessing
{
    public Process(data : any) : any
    {
        return data;
    }
}

// when only compression is enabled
class CompressionAdapter : DataProcessing
{
    private compression : Compression;

    public Process(data : any) : any
    {
        return this.compression.Compress(data);
    }
}

// when only encryption is enabled
class EncryptionAdapter : DataProcessing
{
    private encryption : Encryption;

    public Process(data : any) : any
    {
        return this.encryption.Encrypt(data);
    }
}

// when both, compression and encryption are enabled
class CompressionEncryptionAdapter : DataProcessing
{
    private compression : Compression;
    private encryption : Encryption;

    public Process(data : any) : any
    {
        return this.encryption.Encrypt(
            this.compression.Compress(data)
        );
    }
}

By doing this, you end up with 4 classes, each doing something completely different, but each of them providing the same public API. The Process method.

In your business logic, where you deal with the none/encryption/compression/both decision, you will design your object to make it depend on the DataProcessing interface we designed before.

class DataService
{
    private dataProcessing : DataProcessing;

    public DataService(dataProcessing : DataProcessing)
    {
        this.dataProcessing = dataProcessing;
    }
}

The process itself could then be as simple as this:

public ComplicatedProcess(data : any) : any
{
    data = this.dataProcessing.Process(data);

    // ... perhaps work with the data

    return data;
}

No more conditionals. The class DataService has no idea what will really be done with the data when it is passed to the dataProcessing member, and it does not really care about it, it is not its responsibility.

Ideally, you would have unit tests testing the 4 adapter classes you created to make sure they work, you make your test pass. And if they pass, you can be pretty sure they will work no matter where you call them in your code.

So doing it this way I will never have ifs in my code anymore?

No. You are less likely to have conditionals in your business logic, but they still have to be somewhere. The place is your factories.

And this is good. You separate the concerns of creation and actually using the code. If you make your factories reliable (in Java you could even go as far as using something like the Guice framework by Google), in your business logic you are not worried about chosing the right class to be injected. Because you know your factories work and will deliver what is asked.

Is it necessary to have all these classes, interfaces, etc.?

This brings us back to the begining.

In OOP, if you choose the path to use polymorphism, really want to use design patterns, want to exploit the features of the language and/or want to follow the everything is an object ideology, then it is. And even then, this example does not even show all the factories you are going to need and if you were to refactor the Compression and Encryption classes and make them interfaces instead, you have to include their implementations as well.

In the end you end up with hundreds of little classes and interfaces, focused on very specific things. Which is not necessarily bad, but might not be the best solution for you if all you want is to do something as simple as adding two numbers.

If you want to get it done and quickly, you can grab Ixrec's solution, who at least managed to eliminate the else if and else blocks, which, in my opinion, are even a tad worse than a plain if.

Take into consideration this is my way of making good OO design. Coding to interfaces rather than implementations, this is how I have done it for the past few years and it is the approach I am the most comfortable with.

I personally like the if-less programming more and would much more appreciate the longer solution over the 5 lines of code. It is the way I am used to designing code and am very comfortable reading it.


Update 2: There has been a wild discussion about the first version of my solution. Discussion mostly caused by me, for which I apologize.

I decided to edit the answer in a way that it is one of the ways to look at the solution but not the only one. I also removed the decorator part, where I meant facade instead, which I in the end decided to leave out completely, because an adapter is a facade variation.

Andy
  • 10,400
12

I guess your question is looking not for practicality, in which case lxrec's answer is the correct one, but to learn about design patterns.

Obviously the command pattern is an overkill for such a trivial problem as the one you propose but for the sake of illustration here it goes:

public interface Command {
    public String transform(String s);
}

public class CompressCommand implements Command {
    @Override
    public String transform(String s) {
        String compressedString=null;
        //Compression code here
        return compressedString;
    }
}

public class EncryptCommand implements Command {
    @Override
    public String transform(String s) {
        String EncrytedString=null;
        // Encryption code goes here
        return null;
    }

}

public class Test {
    public static void main(String[] args) {
        List<Command> commands = new ArrayList<Command>();
        commands.add(new CompressCommand());
        commands.add(new EncryptCommand()); 
        String myString="Test String";
        for (Command c: commands){
            myString = c.transform(myString);
        }
        // now myString can be stored in the database
    }
}

As you see putting the commands/transformation in a list allows for ther to be executed sequentially. Obviously it will execute both, or only one of them dependind og what you put in the list without if conditions.

Obviously the conditionals will end up in some kind of factory that puts together the command list.

EDIT for @texacre's comment:

There are many ways of avoiding the if conditions in the creational part of the solution, let's take for example a desktop GUI app. You can have checkboxes for the compress and encrypt options. In the on clic event of those checkboxes you instantiate the corresponding command and add it to the list, or remove from the list if you are deselecting the option.

Tulains Córdova
  • 39,570
  • 13
  • 100
  • 156
7

I think "design patterns" are unneccesarily geared towards "oo patterns" and completely avoiding much simpler ideas. What we're talking about here is a (simple) data pipeline.

I would try to do it in clojure. Any other language where functions are first-class is probably ok as well. Maybe I could a C# example later on, but it's not as nice. My way of solving this would be the following steps with some explanations for non-clojurians:

1. Represent a set of transformations.

(def transformations { :encrypt  (fn [data] ... ) 
                       :compress (fn [data] ... )})

This is a map, i.e. a lookup table/dictionary/whatever, from keywords to functions. Another example (keywords to strings):

(def employees { :A1 "Alice" 
                 :X9 "Bob"})

(employees :A1) ; => "Alice"
(:A1 employees) ; => "Alice"

So, writing (transformations :encrypt) or (:encrypt transformations) would return the encrypt function. ((fn [data] ... ) is just a lambda function.)

2. Get the options as a sequence of keywords:

(defn do-processing [options data] ;function definition
  ...)

(do-processing [:encrypt :compress] data) ;call to function

3. Filter all transformations using the supplied options.

(let [ transformations-to-run (map transformations options)] ... )

Example:

(map employees [:A1]) ; => ["Alice"]
(map employees [:A1 :X9]) ; => ["Alice", "Bob"]

4. Combine functions into one:

(apply comp transformations-to-run)

Example:

(comp f g h) ;=> f(g(h()))
(apply comp [f g h]) ;=> f(g(h()))

5. And then together:

(def transformations { :encrypt  (fn [data] ... ) 
                       :compress (fn [data] ... )})

(defn do-processing [options data]
  (let [transformations-to-run (map transformations options)
        selected-transformations (apply comp transformations-to-run)] 
    (selected-transformations data)))

(do-processing [:encrypt :compress])

The ONLY changes if we want to add a new function, say "debug-print", is the following:

(def transformations { :encrypt  (fn [data] ... ) 
                       :compress (fn [data] ... )
                       :debug-print (fn [data] ...) }) ;<--- here to add as option

(defn do-processing [options data]
  (let [transformations-to-run (map transformations options)
        selected-transformations (apply comp transformations-to-run)] 
    (selected-transformations data)))

(do-processing [:encrypt :compress :debug-print]) ;<-- here to use it
(do-processing [:compress :debug-print]) ;or like this
(do-processing [:encrypt]) ;or like this
NiklasJ
  • 675
5

[ Essentially, my answer is a follow-on to the answer by @Ixrec above. ]

An important question: is the number of distinct combinations that you need to cover is going to grow? You are better aware of your subject domain. This is your judgement to make.
Can the number of variants possibly grow? Well, that's not inconceivable. For example, you may need to accommodate more different encryption algorithms.

If you anticipate that the number of distinct combinations is going to grow, then Strategy pattern can help you. It's designed to encapsulate algorithms and provide an interchangeable interface to the calling code. You would still have a small amount of logic when you create (instantiate) the appropriate strategy for each particular string.

You have commented above that you don't expect the requirements to change. If you don't expect that the number of variants would grow (or if you can defer this refactoring), then keep the logic the way it is. Currently, you have a small and manageable amount of logic. (Maybe put a note to self in the comments about a possible refactoring to a Strategy pattern.)

Nick Alexeev
  • 2,532
1

One way to do this in scala would be:

val handleCompression: AnyRef => AnyRef = data => if (compressEnable) compress(data) else data
val handleEncryption: AnyRef => AnyRef = data => if (encryptionEnable) encrypt(data) else data
val handleData = handleCompression andThen handleEncryption
handleData(data)

Using decorator pattern to achieve the above goals (separation of individual processing logic and how they wire together) would be too verbose.

Wherein you would require a design pattern to achieve these design goals in an OO programming paradigm, functional language offers native support by using functions as first class citizens (line 1 and 2 in the code) and functional composition (line 3)