Suggested File Transfer API

[SoniEx2]

So I heard Terasology has an in-game computers mod. It would be cool to be able to save files from those computers onto my hard drive and send files from my hard drive onto those computers.

The idea I had for this was 2 simple client API methods:

Optional<InputStream> loadFile();
Optional<OutputStream> saveFile(String);

Where loadFile() opens a file selection dialog for opening a file and saveFile opens a file selection dialog for saving a file.

If the user cancels or closes the file selection, or if there's an error opening the file, then loadFile() and saveFile() return empty.

Why use InputStream and OutputStream instead of byte[]? Well it's simple, first InputStream and OutputStream are lazy, which means you won't read the whole file at once (e.g. for 2GB files this could take quite a while), and second because that's the Java way to do it. That way, the mod can control how fast to read and how much to read, as well as how to buffer and stuff.

To prevent wearing out the user's HDD and stuff, seeking aka mark/reset should be disallowed - wrap the returned InputStream in a BufferedInputStream if you need that.

saveFile() takes a filename, not a path. Any path separators and stuff should be stripped from this filename.

While that's my idea for how the API should work, I'm not entirely sure on what the methods should be called... Does loadFile() and saveFile() sound good? They *are* client methods after all, so they don't really transfer any files - they only allow the transfer of files.

 

[Florian]

As I said in the IRC chat already, I am not sure if I would knew instantly when I read "loadFile" that this will result in a select file dialog to open. Although it depends on the context. e.g. fileSelector.loadFile() would also be explaining enough.

It might also be better to have errors be handled via exceptions, as you then know what happened and can log it. Old java file API returns false at errors, new one throws exceptions.

Also as an idea to throw into the room: The file selection could be done via a method call selectFile that returns a object that wrapps a file and just offers openOutputStream and openInputStream methods + some utility methods like readAllLines. That would have the advantage that
actually using the API becomes easier. Since opening a stream means also that you need to have a try-block that ensures that the stream gets closed. By separating the file selection and file opening the code becomes easier to understand (the file selection is more clearer) and simpler as the new try-block can be used:

Optional<SelectedFile> optionalFile = fileManager.selectFile();
if (!optionalFile.isPresent()) {
   return; // or some other handling
}
SelectedFile file = optinalFile.get();
String text;
try (InputStream inputStream = file.openStream()) {
    text = readInputStreamAsString(inputStream);
}

I suggest that you as next step create a pull request that adds this feature and simultaneously a pull request that makes a module use that feature. The latter PR can then be used to verify that the first PR works. Further details can then be discussed in the pull requests.

 

[Cervator]

@SoniEx2: At this point I would just go for a prototype implementation if you're interested in moving forward with this. For such an implementation the naming doesn't matter one bit. Your Stream classes are on the whitelist now - use them for something!

Working code makes a world of a difference when then reviewing and considering the exact options and naming. Renaming things during the PR stage is easy.

Also, don't focus overly much on optimization until you know you actually have a performance issue. Just go with a sane and understandable approach without diving straight into edge cases. That's another things that's very easy to deal with in an initial PR.

In short: write some code already!