tags:

views:

1468

answers:

12

Hi, I've coded a method something like this... But I guess this should undergo refactoring. Can any one suggest the best approach to avoid using this multiple if statements?

private String getMimeType(String fileName){
if(fileName == null) {
    return "";   
} 
if(fileName.endsWith(".pdf")) {
    return "application/pdf";   
}
if(fileName.endsWith(".doc")) {
    return "application/msword";  
}
if(fileName.endsWith(".xls")) {
    return "application/vnd.ms-excel"; 
}
if(fileName.endsWith(".xlw")) {
    return "application/vnd.ms-excel"; 
}
if(fileName.endsWith(".ppt")) {
    return "application/vnd.ms-powerpoint"; 
}
if(fileName.endsWith(".mdb")) {
    return "application/x-msaccess"; 
}
if(fileName.endsWith(".rtf")) {
    return "application/rtf"; 
}
  if(fileName.endsWith(".txt")) {
    return "txt/plain"; 
 }
  if(fileName.endsWith(".htm") || fileName.endsWith(".html")) {
    return "txt/html"; 
 }
   return "txt/plain"; 
}

I' cannot use switch-case here as my 'condition' here is a java.lang.String !!

thanks, Hanu

+10  A: 

Using a HashMap perhaps?

This way you could do myMap.get(mystr);

Kezzer
but he needs `endsWith`, `get` uses an exact match
dfa
@dfa: he's matching a file suffix ... stuff after the last ".".
Stephen C
@Stephen C: exactly, you can just get everything after the last "." by using things like `LastIndexOf`.
Kezzer
@Stephen: `myMap.get(".pdf) != fileName.endsWith(".pdf")`; he needs at least to extract the extension first
dfa
@dfa: I think we can assume that the OP has the intelligence to figure out how to take the suffix a String.
Stephen C
A: 

You can always use a Groovy class here as it allows for switch-case on Strings :)

Shimi Bandiel
A: 

How about mapping the extensions to MIME types, then using a loop? Something like:

Map<String,String> suffixMappings = new HashMap<String,String>();
suffixMappings.put(".pdf", "application/pdf");
...

private String getMimeType(String fileName){
    if (fileName == null) {
        return "";   
    }
    String suffix = fileName.substring(fileName.lastIndexOf('.'));
    // If fileName might not have extension, check for that above!
    String mimeType = suffixMappings.get(suffix); 
    return mimeType == null ? "text/plain" : mimeType;
 }
Sean Owen
Why bother with a Map if you're simply going to loop over it? Would be better to strip the suffix and perform an ~O(1) direct look-up.
Adamski
a better strategy here is to extract the extension (filename.ext -> ext) and do a lookup with suffixMappings
dfa
for iterating it would probably better to loop over the entrySet() in order to avoid the second lookup by get().
MartinStettner
Yeah you guys are right, would make more sense to just pick off the suffix. In this case it seems it can reliably be determined.Also agree with the entrySet() optimization. Hmm, wonder if I can edit this to incorporate...
Sean Owen
+1  A: 

There is no way to evade that in general. In your case - if there is a set of allowed extensions - you could create an Enum, convert the extension to the Enum type via valueOf(), and then you can switch over your enum.

Zed
Yeah... I would go for those HashMaps too, now that I thought it over :)
Zed
+23  A: 
Map<String,String> extensionToMimeType = new HashMap<String,String>();
extensionToMimeType.put("pdf", "application/pdf");
extensionToMimeType.put("doc", "application/msword");
// and the rest

int lastDot = fileName.lastIndexOf(".");
String mimeType;
if (lastDot==-1) {
  mimeType=NO_EXTENSION_MIME_TYPE;
} else {
  String extension = fileName.substring(lastDot+1);
  mimeType=extensionToMimeType.get(extension);
  if (mimeType == null) {
    mimeType=UNKNOWN_EXTENSION_MIME_TYPE;
  }
}
Joachim Sauer
+1 for being complete
dfa
@dfa: too much example code is copy-and-pasted ;-)
Joachim Sauer
paste results in compilation errors... can't find NO_EXTENSION_MIME_TYPE or UNKNOWN_EXTENSION_MIME_TYPE. Plz Hlp :-)
extraneon
@extraneon: those are left as an exercise for the reader and depend on the specification.
Joachim Sauer
+1 for providing a complete example.
Grant Wagner
@extraneon: Add `private static final NO_EXTENSION_MIME_TYPE = "application/octet-stream";` and `private static final UNKNOWN_EXTENSION_MIME_TYPE = "application/octet-stream";` to the top of your class. Change `application/octet-stream` to whatever **you** want to use for the *no extension* and *unknown* MIME types.
Grant Wagner
subtring should be *substring* above
Roalt
A: 

I would do this by putting the associations in a map, and then using the map for lookup:

Map<String, String> map = new HashMap<String, String>();

map.put(".pdf", "application/pdf");
map.put(".doc", "application/msword");
// ... etc.

// For lookup:
private String getMimeType(String fileName) {
    if (fileName == null || fileName.length() < 4) {
        return null;
    }

    return map.get(fileName.substring(fileName.length() - 4));
}

Note that using the switch statements on strings is one of the proposed new features for the next version of Java; see this page for more details and an example of how that would look in Java 7:

switch (fileName.substring(fileName.length() - 4)) {
    case ".pdf": return "application/pdf";
    case ".doc": return "application/msword";
    // ...
    default: return null;

(edit: My solution assumes the file extension is always 3 letters; you'd have to change it slightly if it can be longer or shorter).

Jesper
A: 

Create an enum called MimeType with 2 String variables: extension and type. Create an appropriate constructor and pass in the ".xxx" and the "application/xxx" values. Create a method to do the lookup. You can use enums in switch.

aberrant80
+3  A: 

Personally I don't have problems with the if statements. The code is readable, it took just milliseconds to understand what you're doing. It's a private method anyway and if the list of mime types is static then there's no urgent need to move the mapping to a properties file and use a lookup table (map). Map would reduce lines of code, but to understand the code, then you're forced to read the code and the implementation of the mapping - either a static initializer or an external file.

You could change the code a bit and use an enum:

private enum FileExtension { NONE, DEFAULT, PDF, DOC, XLS /* ... */ }

private String getMimeType(String fileName){
  String mimeType = null;

  FileExtension fileNameExtension = getFileNameExtension(fileName);

  switch(fileNameExtension) {
    case NONE:
      return "";
    case PDF:
      return "application/pdf";

    // ...

    case DEFAULT:
      return "txt/plain";   
  }

  throw new RuntimeException("Unhandled FileExtension detected");
}

The getFileNameExtension(String fileName) method will just return the fitting enum value for the fileName, FileExtension.NONE if fileName is empty (or null?) and FileExtension.DEFAULT if the file extension is not mapped to a mime type.

Andreas_D
+4  A: 

what about using a MIME detection library instead?

  • mime-util
  • mime4j
  • JMimeMagic library - Free. Uses file extension and magic headers to determine MIME type.
  • mime-util - Free. Uses file extension and magic headers to determine MIME type.
  • DROID (Digital Record Object Identification) - Free. Uses batch automation to detect MIME types.
  • Aperture Framework - Free. A framework for crawling external sources to identify MIME types.

(feel free to add more, there so many libraries..)

dfa
A: 

Just to mention it: A direct equivalent to your code would not be using a map for direct lookup (since that would require each extension to have exactly 3 characters) but a for loop:

...
Map<String, String> extmap = GetExtensionMap();
for (Map.Entry<String,String> entry: extmap.entrySet())
  if (fileName.endsWith(entry.getKey))
    return entry.getValue();
...

This solution works with extensions of any length but is less performant than the hash lookup of course (and slightly less performant than the original solution)

The Algorithmic-Design-Guy solution

A more performant way would be to implement a tree structure starting with the last character of the extension and storing the appropriate MIME types at the respective nodes. You could then walk down the tree starting with the last character of the file name. But this is probably an overkill ...

MartinStettner
+2  A: 

I consider your approach to be the best overall. This comes after having tested with a number of different approaches myself.

I see a number of huge benefits in your current approach, namely:

  1. Easily readable and understandable by anyone (in my experience, medium-level programmers often underestimate this and usually prefer going with fancy-patterns which, in the end are not readable at all for the vast majority of programmers who do not know that specific pattern)
  2. All the information is in one single place. As Andreas_D pointed out, hunting around files or classes is not a good option for someone that needs to fix a bug while you are on holiday!
  3. Easily maintainable: I could "F3" (if you are Eclipse-ing) on the method and add a new content type in seconds without any worries of introducing bugs!

I can suggest a few things anyway:

  1. This method is very general purpose: Why should it be private?! This is a public method of some utility/helper class! Moreover it should be a static method!! You don't need anything from the Object itself to perform your job!
  2. You could use indenting to make things prettier and compact. I know that indenting is some kind of religion for the most of us, but I think it should not be a strict rule; it should be properly used to make our code more readable and compact. If this would be a config file you would probably have something like:
pdf=application/pdf
doc=application/msword

You could have a very similar result with:

    public static String getMimeType(String fileName){
       if(fileName == null) return "";
       if(fileName.endsWith(".pdf")) return "application/pdf";
       if(fileName.endsWith(".doc")) return "application/msword";
       if(fileName.endsWith(".xls")) return "application/vnd.ms-excel"; 
       return "txt/plain"; 
   }

This is also what a lot of the Map based implementations look like.

Pokot0
Your code would look better if it used the ternary operator.
Tom Hawtin - tackline
A: 

Easiest and shortest way for this particular problem would be using the builtin Java SE or EE methods.

Either in "plain vanilla" client application (which derives this information from the underlying platform):

String mimeType = URLConnection.guessContentTypeFromName(filename);

Or in a JSP/Servlet web application (which derives this information from the web.xml files):

String mimeType = getServletContext().getMimeType(filename);
BalusC