Java code refactoring: multiple instanceof operator usages

Question

Consider the following example where the classes TextFile, XmlFile, HtmlFile, ShellScriptFile, et cetera, are all subclasses of the class SimpleFile. I am writing a class FileOperations, which has a method that searches the contents of a generic file depending on its type.

Following is a code sample:

public searchFile(SimpleFile targetFile, String searchStr) {
     if (targetPage instanceof HtmlFile) {
         // search html file
     }
     else if (targetPage instanceof TextFile) {
         // search text file
     }
     else if (targetPage instanceof XmlFile) {
         // search xml file
     }
     else if (targetPage instanceof ShellScriptFile) {
         // search shell file
     }   
     ...
}

This structure smells bad to me. I am aware that this is the best case for polymorphism. But I do not have control over the File class or its subclasses.I cannot write to them.

Is there another way to go about cleaning this mess? Because the if-else structure is going to keep increasing as I add support for different file types.

Else, if I am stuck with this structure, then is instanceof the fastest operator in Java? What are its implications on performance?

Would it be better to use getClass() or isAssignableFrom() for the above case?

I appreciate your comments or suggestions!

Answer 1

First I would say that your example doesn't seem bad at all. It's a little verbose, and arguable not as cohesive as separating them into different methods but it looks fairly normal to me. I think a neater approach might be to use overloading instead. I can't speak for the difference in speed but from a maintenance and extensibility stance it will be easier to deal with in future.

public void searchFile(SimpleFile targetFile , String searchStr) {
     // SimpleFile generalized behavior.
     if (targetPage instanceof HtmlFile) searchFile((HtmlFile)targetFile, searchStr);
     else if (targetPage instanceof TextFile) searchFile((TextFile)targetFile, searchStr);
     else if (targetPage instanceof XmlFile) searchFile((XmlFile)targetFile, searchStr);
     else if (targetPage instanceof ShellScriptFile) searchFile((ShellScriptFile)targetFile, searchStr);
     else System.out.println("Subtype not recognised"); 
}
public void searchFile(HtmlFile targetFile , String searchStr) {
    // HtmlFile specific behavior
}
public void searchFile(TextFile targetFile , String searchStr) {
    // TextFile specific behavior
}
public void searchFile(XmlFile targetFile , String searchStr) {
    // XmlFile specific behavior
}
public void searchFile(ShellScriptFile targetFile , String searchStr) {
    // ShellScript specific behavior
}

In the event of a new subclass of SimpleFile, it will default to the SimpleFile version of the method. In the event that the type is not known at compile time (as brought up in the comments) the most general searchFile() method can be used to check and redistribute the object accordingly.

Edit: As a note on the performance impact of multiple use of instanceof, the consensus appears to be that it's irrelevant, and that the use of modern instanceof is pretty fast anyway.