112

My code here detects if the mimeType is equals to some MIME type, if it is, it will do a certain conversion

public void convertToMp3(File src, File target,String mimeType){
    if(mimeType.equals("audio/mpeg")){
        ...
    }else if(mimeType.equals("audio/wav")){
        mp3ToWav();
    }else if(mimeType.equals("audio/ogg")){
        ...
    }else if(...){
    ... //More if and else here
}

I have shortened my code, because it has a lot of else if statements, What design pattern is suitable for removing many if and else or else if statements?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
user962206
  • 15,637
  • 61
  • 177
  • 270
  • 11
    How about enums and switch? – Pradeep Simha Jan 03 '13 at 10:13
  • 1
    The factory design pattern doesn't suit your needs? – Roman C Jan 03 '13 at 10:21
  • 1
    @RomanC why would I use a factory design pattern? – user962206 Jan 03 '13 at 10:25
  • You want to simplify the code removing many if-else statements, I used for it the factory pattern. Allowing that a polymorphism to work for you. – Roman C Jan 03 '13 at 10:26
  • @RomanC can you provide an example? – user962206 Jan 03 '13 at 10:32
  • 13
    Is there something wrong with if/else statements? Sure, it's not a beautiful, over-engineered OOP pattern but it is straightforward and easily maintainable. Anybody reading it can understand it, so why try to convert it to something else? – laurent Jan 03 '13 at 13:25
  • @user962206 http://en.wikipedia.org/wiki/Facade_pattern All converters can be wrapped inside a facade. Then you call facade.convert() –  Jan 03 '13 at 18:49
  • @user962206 the answer of the user pgras - http://stackoverflow.com/a/14137017/1466267 – SpaceTrucker Jan 04 '13 at 17:44
  • @this.lau_ it is not maintainable.it is a shame – giorgi dvalishvili Aug 26 '17 at 17:58
  • @laurent Really, nothing wrong with the if/else solution. though if too much code starts to accumulate on each of the branches, things start getting more difficult to maintain/understand. So, it is a matter of code-taste/own-understanding. The "Converter" example below, with a hashmap is handy for me. Whether they name it "Strategy Pattern" or "Command Pattern"... It seem both patterns have been named for the same thing, in chosen answer comments and in the selected answer of "This question already has answers here". – nephewtom Nov 12 '21 at 13:39
  • Yes, 8 years later, I actually agree. Especially for something like mime types, of which there are many, I'd use something other than if/else today. – laurent Nov 13 '21 at 09:48

7 Answers7

193

You could have a Converter interface. Then you could create a class for each Mimetype like:

public interface Converter {

    public void convertToMp3();
    public void convertToOgg();

}

public class MpegConverter implements Converter {

    public void convertToMp3() {
        //Code here
    }

    public void convertToOgg() {
        //Code here
    }

}

You would need a class like this for each converter. Then you could set up a map like this:

Map<String, Converter> mimeTypeMap = new HashMap<String, Converter>();

mimeTypeMap.put("audio/mpeg", new MpegConverter());

Then your convertToMp3 method becomes like this:

Converter converter = mimeTypeMap.get(mimeType);
converter.convertToMp3();

Using this approach you could easily add different converters in the future.

All untested, probably doesn't compile, but you get the idea

cowls
  • 24,013
  • 8
  • 48
  • 78
  • but what if I have different converters other than convertingToMp3? I also have converter for Ogg and Wav. I feel kinda lost though, whati f I want to add an Ogg Converter – user962206 Jan 03 '13 at 10:27
  • You can add additional methods to the interface. e.g. convertToOgg() Ive updated answer – cowls Jan 03 '13 at 10:29
  • but you are missing the new Method in the class ;) – SomeJavaGuy Jan 03 '13 at 10:31
  • Finally Got it! With this I can convert any audio file in an instance! if ever my management team decided to add another file conversion all I need is to create another Converter Class! Thanks! – user962206 Jan 03 '13 at 10:36
  • +1 - I would add the ability forte converter to determine if can handle a given mi e type, that way you could have a registry of converters, you could use the registry to return a convert(s) that is capable of handling the mime type, something similar to the way that the ImageIO API works - for example – MadProgrammer Jan 03 '13 at 10:49
  • Why having convertToMp3,convertToOgg in the Converter interface? They are basically doing the same job. Why not having one interface Converter with only one method, and provide correct implementation for each conversion? – Dimitri Jan 03 '13 at 10:58
  • 2
    In this example convertToMp3 method is used on the MPEGConverter to convert from MPG to MP3, convertToOgg is used to convert from MPG to OGG. So no, they are not doing the same job. Possible confusion with the naming – cowls Jan 03 '13 at 10:59
  • 3
    Good idea, looks like a very clean solution to me. Just do not forget to check if there actually is a handler and give an error message if no handler can be found. – Alexander Weinert Jan 03 '13 at 14:25
  • @cowls I think you can't have different methods in your `MpegConverter` like - `convertToMp3()` and `convertToOgg()`. Because when you retrieve `MpegConverter` class from HashMap, how can you know which method to call? So as far as I understand you can only have 1 handler method per class, like `convert(File src, File target)`, just like Raedwald said. Otherwise those are ifs again: `if (mimeType.equals("audio/ogg")) converter.convertToOgg(); else if (mimeType.equals("audio/mp3")) converter.convertToMp3();` etc... Please correct me if I am wrong. – randomUser56789 Jan 30 '13 at 21:48
  • Hi @azxc123. I dont think you quite have it, the calling method needs to know whether it wants to convert to an mp3 or ogg or wav. Then it can call the corresponding method on the Converter. – cowls Jan 30 '13 at 22:18
  • 10
    For those who may not know, this is called the [Strategy Pattern](https://en.wikipedia.org/wiki/Strategy_pattern). – Andrew Marshall May 25 '13 at 18:06
  • and if different Converters has different constructors (1-3 params) which are not known at time of creating HashMap but just before conversion? – Ewoks Apr 26 '16 at 20:01
23

If you use pre-JDK7, you may add an enum for all MIME types:

  public static enum MimeTypes {
      MP3, WAV, OGG
  }

  public class Stuff {
      ...
      switch (MimeTypes.valueOf(mimeType)) {
          case MP3: handleMP3(); break;
          case WAV: handleWAV(); break;
          case OGG: handleOGG(); break;
      }
  }

And have a look at the Stack Overflow question Java - Convert String to enum on how to convert Strings to enums.

Community
  • 1
  • 1
pgras
  • 12,614
  • 4
  • 38
  • 46
  • 15
    Why don't you put a handle method at the enum itself? Then it would be `MimeTypes.valueOf(mimeType).handle()`. – SpaceTrucker Jan 03 '13 at 11:15
  • @SpaceTrucker: good suggestion, it would be cleaner than my idea. In fact it would be an good implementation of cowls idea because there would be no need to handle a Map with the different implementations – pgras Jan 04 '13 at 09:04
  • 4
    I can't believe that a response which suggests using `switch` as a way of refactoring multiple `if else if` got 23 upvotes. Grrr...... it makes me fear when I think what kind of developers are out there. – Daniel Jul 11 '14 at 14:48
  • @Daniel Well, I guess too many developers think that "fewer lines = better" even though the newer code shares the same disadvantages, than the old one. – Tom Oct 11 '15 at 17:03
  • 1
    @Daniel, could you be more explicit with your concerns about this approach? The use case provided in the question boils down to "execute one-and-only-one code path based on an input variable", which is (in my opinion) a pretty good definition of exactly what a switch is supposed to do. – scubbo Feb 03 '17 at 01:05
15

Consider using the Strategy design pattern and a Map to dispatch to the appropriate strategy. Particularly useful if you you will need additional functionality, in addition to a conversion for a particular mimeType, or the convertors are large and complicated code and you would want to place each convertor in its own .java file.

 interface Convertor {
    void convert(File src, File target);
 }

 private static void convertWav(File src, File target) {
    ...
 }

 ...

 private static final Map< String, Convertor > convertors = new ...;
 static {
    convertors.put("audio/wav", new Convertor {
       void convert(File src, File target) {
          convertWav(src, target);
       }
    });
    convertors.put("audio/ogg", new Convertor {
       void convert(File src, File target) {
          convertOgg(src, target);
       }
    });
    ...
 }

 public void convertToMp3(File src, File target, String mimeType){
     final Convertor convertor = convertors.get(mimeType);
     if (convertor == null ) {
        ...
     } else {
        convertor.convert(src, target);
     }
 }
Raedwald
  • 46,613
  • 43
  • 151
  • 237
3

If you run the same methods for each case you should check State pattern

Marcin Szymczak
  • 11,199
  • 5
  • 55
  • 63
  • I run different methods. – user962206 Jan 03 '13 at 10:25
  • 6
    State pattern is not what you need here, this isn't modelling a finite state machine it's selecting a different conversion strategy based on the input type so the strategy pattern is more appropriate. – Paolo Jan 03 '13 at 13:55
2

If you are using JDK 7, you can use switch-case construct:

See: Why can't I switch on a String?

For prior versions, if-else is the only choice.

Community
  • 1
  • 1
Azodious
  • 13,752
  • 1
  • 36
  • 71
2

It's definitely a Strategy design pattern. But you have a big problem in your general design. It's not a good programming habit to use String to identify a type. Simply because it's easily editable and you can make a grammar mistake and spend all the afternoon looking for a programming mistake. You can avoid using map<>.

I suggest the following:

  1. Extend class File. The new class adds a new attribute FileType and a new method convertTo(FileType) to class File. This attribute holds its type: “audio” , “wav”... and again don't use String, Use Enum. In this case I called it FileType. Extend File as much as you want: WavFile, AudioFile...
  2. Use a Strategy dp to create your converters.
  3. Use a Factory dp to initialize the converters.
  4. Since every File knows its own type and the target type (use convertTo() method to specify the target type) it will call the factory to get the correct Converter automatically!!!

This design is scalable and you can add as much as you need FileType and converters. The answer you vote for is misleading!!!! There is a big difference between coding and hacking.

george
  • 139
  • 1
  • 4
0

If you are not using Java 7 you could create an enum and use that value with a switch case. You then only need to pass the enum value (rather than a file, I don't why you are doing that). It would look neater too.

These should help with what you want to do:

 [Java Enum Examples][1] - 
 [Java Switch Case examples][2]
Skepi
  • 478
  • 3
  • 12