Java application that support all external files

Question

I am doing an application that support external files with different extensions. With a FileChooser the user selects the file, I get the name of the file and the extension. And now is the problem: if, for example, the user choose an xml, I need to get the nodes, if he chooses an CSV file I need to do split, if he ...

I know that use something like that is a bad programming

if(extension==".xml"){
  XMLImportFile();
}else if (extension==".txt"){
   TXTImportFile();
} else if (extension==".csv"){
 ...
}

So what you suggest? I was thinking using an interface (I don't know it the best idea and how it should be used) or other thing, if necessary, but I really want to avoid bad programming.

To start off: read [this](http://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java#513839) before you write any further java-code. As for "bad programming": as far as I can see this is just a plain `if-else` structure. There sure are other ways to do this, but there's nothing bad about `if`-clauses. You should clarify what you even want. I can't find a single question-mark in the whole "question". — , Jun 04 '16 at 12:22

Sergey Kalinichenko · Accepted Answer · 2016-06-04T12:52:54.537

3

A chain of ifs does not necessarily make it a bad code. When you need to do multiple different things based on external input, sometimes multiple ifs is a way to go. The code that does the actual parsing is bound to be different, mirroring the differences in file format.

The thing that you could make slightly more optimal is getting to the actual processing code. Make an interface FileProcessor for processing a file

interface FileProcessor {
    void importFile(String fileName);
}

make implementations of this interface for different extension types, e.g. TxtFileProcessor, CsvFileProcessor, XmlFileProcessor, and put its implementations in a Map<String,FileProcessor> organized by file extension:

Map<String,FileProcessor> extensionToProcessor = new HashMap<>();
...
extensionToProcessor.put("txt", new TxtFileProcessor());
extensionToProcessor.put("xml", new XmlFileProcessor());
extensionToProcessor.put("csv", new CsvFileProcessor());

This would let you unify the dispatch, like this:

FileProcessor proc = extensionToProcessor.get(fileExtension);
if (proc != null) {
    proc.importFile(fileName);
} else {
    throw new UnknownFileException("Files of type '"+fileExtension"' are not supported.");
}

edited Jun 04 '16 at 12:52

answered Jun 04 '16 at 12:33

Sergey Kalinichenko

714,442
84
1,110
1,523

1

"import" is a reserved keyword and cannot be used as method name. Also your code snippets look like the processors are supposed to be reuseable, thus they should be stateless/ return the result immediately. – ST-DDT Jun 04 '16 at 12:44
@dasblinkenlight Thanks dasblinkenlight , I will adopt that strategy, the use of the interface and the fileProcessor seems the more correct. You have my UpVote! – Joseph Jun 04 '16 at 13:28
@dasblinkenlight Just one more thing ... instead of "txt", "xml" or "csv", (on ´the map)can i use an Enum, and then call .put(FileEnum.TXT ,....) ? – Joseph Jun 04 '16 at 14:16
@Joseph Sure, you could do that, but then you would need to construct an `enum` instance back from `fileExtension` string. You could also associate an extension string with your `enum`, give it a getter accessor method, and put `FileEnum.TXT.extension()` into the map. – Sergey Kalinichenko Jun 04 '16 at 14:35

score 2 · Answer 2 · answered Jun 04 '16 at 12:33

2

Your code isn't that ugly as you think, but I agree with you that it could be made better.

I suggest you to use the strategy pattern aka interface pattern. https://en.wikipedia.org/wiki/Strategy_pattern

public interface FileImporter {

    public YourReturnType read(File file);

}

Please note that FileImporter is supposed to be stateless. And then use it like this:

Map<String, FileImporter> fileMapping = new HashMap<>();
fileMapping .put("xml", new XMLFileImporter());
fileMapping .put("csv", new CSVFileImporter());
// More file mappings ...

FileImporter importer = fileMapping.get(fileSuffix);
if (importer == null) {
   throw new UnsupportedFileException();
}
YourReturnType result = importer.read(file);
// do some stuff with it...

answered Jun 04 '16 at 12:33

ST-DDT

2,615
3
30
51

1

The only thing I would change is the use of enum (ie: FileType.XML.getExtension() and FileType.XML.getImporter()) – alexbt Jun 04 '16 at 12:37
If you have only a single place where the import happens then creating a new interface may be overhead (AFAICT there is none in plain java). But it is also a good location to put the file-extension detection. I won't add the enum here to keep the example short and simple. – ST-DDT Jun 04 '16 at 12:50
@ST-DDT Thank you ST-DDT for that approach, i'll also read more about the Strategy Pattern – Joseph Jun 04 '16 at 13:32

score -1 · Answer 3 · answered Jun 04 '16 at 12:29

-1

One of the solutions could be Command pattern. Have a look at https://en.wikipedia.org/wiki/Command_pattern#Java_8

answered Jun 04 '16 at 12:29

Petr

1,159
10
20

Please provide an explanation/description to anything you link to – Jun 04 '16 at 12:31

Java application that support all external files

3 Answers3