Is this file I/O utility efficient or is there a lot of fat?

This is a class/method that manages files being written to and read from. I have a lot of code, and it grew as I tried to get it to work properly. Is there a better way to write this (i.e. trim the fat) or is it written properly?

```
import java.io.*;

public class DataBase {
public static String getExtension(File f) {
String ext = null;
String s = f.getName();
int i = s.lastIndexOf('.');

if (i > 0 && i 0) {
if (line.contains("")) {
line = line.replaceAll("", " ");
String tempLine = "";
while ((tempLine.trim().length() < 1)
&& bStream.available() != 0) {
tempLine = bReader.readLine();
}
line = line + tempLine;
}
buff.append(line += "\n");

}
}

fStream.close();
bStream.close();
bReader.close();

} catch (FileNotFoundException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
}
return buff.toString();

}

public static boolean writeToFile(String fileName, String data,
boolean append) {
boolean isWrite = false;
int dirIndex = fileName.lastIndexOf(getPathSeparator());
if (dirIndex != -1) {
String dir = fileName.substring(0, dirIndex) + getPathSeparator();
java.io.File fDir = new java.io.File(dir);
if (!fDir.exists()) {
if (!fDir.mkdirs()) {
return false;
}
}
}
try {
java.io.FileOutputStream fout = new java.io.FileOutputStream(
fileName, append);
java.nio.channels.FileChannel fChannelWriter = fout.getChannel();
byte[] bytesToWrite = data.getBytes();
java.nio.ByteBuffer bBuffW = java.nio.ByteBuffer.wrap(bytesToWrite);
fChannelWriter.write(bBuffW);
fChannelWriter.close();
fout.close();

DataBase is a poor name for a collection of file I/O convenience routines. Especially one whose readFile() function swallows empty lines and strips
tags as it reads. What exactly are you trying to accomplish with this class? I suspect that you have decomposed your problem poorly, and this class shouldn't exist.

The getExtension() function could be more succinct.

public static String getExtension(File f) {
    String s = f.getName();
    int i = s.lastIndexOf('.');

    return (i > 1 && i < s.length() - 1) ?
        s.substring(i + 1).toLowerCase() :
        null;
}

I've changed the condition from i > 0 to i > 1 since the names of Unix dotfiles (e.g. .profile) are not usually thought of as file extensions. That's a judgement call for you to make.

I think your readFile() should take a File rather than a String parameter. If you design your API so that you consistently use File to represent file paths and String to represent file content data, then it will be harder to accidentally call your function with swapped arguments (especially for writeToFile()). Also, if you call the other function writeToFile(), then you should rename readFile() to readFromFile() for consistency.

Swallowing exceptions is usually a bad idea, even if you print the stack trace. I would just let any IOExceptions propagate.

You're just using the default character encoding, so you don't need to bother with constructing InputStreams — just use Readers.

Testing for String.contains() before doing String.replaceAll() just repeats the work needlessly. String.replaceAll() will just return the original string anyway if no replacement needs to be performed.

I don't know what you are trying to accomplish with the tempLine stuff. Since you initialize tempLine = "", it will always be true that tempLine.trim().length() is 0, so all of that is dead code.

Assuming you don't need thread safety, just use StringBuilder instead of StringBuffer.

Doing a string concatenation in buff.append(line + "\n") defeats the efficiency gains of using a StringBuffer.

All together…

public static String readFromFile(File f) throws IOException {
    if (!f.exists()) {
        // I find this behavior surprising; I would have expected a
        // FileNotFoundException to be thrown instead.
        return "";
    }

    FileReader r = new FileReader(f);
    BufferedReader br = null;
    try {
        br = new BufferedReader(r);
        StringBuilder buff = new StringBuilder();

        String line;
        while (null != (line = br.readLine())) {
            if (line.length() > 0) {
                line = line.replaceAll("", " ");
                buff.append(line).append('\n');
            }
        }
        return buff.toString();
    } finally {
        if (br != null) br.close();
        r.close();
    }
}

The code could be a bit less clumsy if you use Java 7's try-with blocks.

writeToFile() can be simplified by a lot. For simplicity, I would just use a FileWriter instead of the java.nio API.

public static void writeToFile(File f, String data, boolean append) throws IOException {

    File parent = f.getParentFile();
    if (!parent.exists()) {
        parent.mkdirs();
    }

    FileWriter w = new FileWriter(f, append);
    try {
        w.write(data);
    } finally {
        w.close();
    }
}

With the simplification to writeToFile(), getPathSeparator() might not be necessary anymore.

StackExchange Code Review Q#32736, answer score: 3