Storing .h files in SQLite using SQLJet

Question

I’m trying to learn as much as I can on my own by reading lots of examples, documentations, and asking here. I would like to improve my style to write efficient code and adhere to Java standards.

In this small sample of code I would like to get feedback on a few things:

• Exception throwing
• Opening/closing database connection
• Any other comments in general style

There are two classes, Database and Main.

My Database class:

public class Database {

    private String dbName = "";
    private SqlJetDb db = null;

    public Database(String dbName) {
        this.dbName = dbName;
    }

    public void CreateDatabase() throws SqlJetException {...}

    public void OpenDatabaseConnection() throws SqlJetException {...}

    public void CloseDatabaseConnection() throws SqlJetException {...}

    private void InsertRecord(String file) throws SqlJetException {...}

    public void GetDirectoryContent(String dir) {
        File directory = new File(dir);
        if (directory.isDirectory()) {
            String[] content = directory.list();
            for (String s : content) {
                GetDirectoryContent(dir + "\\" + s);
            }
        } else {
            String file = directory.toString();
            String extension = file.substring(file.lastIndexOf("."));
            if (extension.equals(".h")) {
                try {
                    InsertRecord(file);
                } catch (SqlJetException e) {
                    e.printStackTrace();
                }
            }
        }
    }
}

Call in main:

Database db = new Database("test.db");
    try {
        db.CreateDatabase();
        db.OpenDatabaseConnection();
        db.GetDirectoryContent("C:\\test");         
    } catch (SqlJetException e) {
        e.printStackTrace();
    } finally {
        try {
            db.CloseDatabaseConnection();
        } catch (SqlJetException e) {
            e.printStackTrace();
        }           
    }
}

Answer 1

Personally I would

• always log rather than use e.printStackTrace. You can log to console if you require. By logging you can then change it easily to file logging and include your own comments
• use File.seperator instead of \. This gives a system independent way of separating your file paths.
• Consider making dbName final. You do not need to initialise it first and then in the ctor. Just do it in the ctor. Things like dbName should not be mutable.

Answer 2

Consider in the following:

public Database(String dbName) {
    this.dbName = dbName;
}

1. I would check for a null dbName being passed in. Note that you pass this reference and then simply store it. Consequently, if it's null, you won't find out until later (perhaps, much later). You then have to work out at what point that was set to null.
2. Will you change dbName ? If not, make it final. It'll stop you changing it inadvertently later on. Immutability is often a good idea. It makes the class more robust and thread-safety easier to achieve. Perhaps not a requirement here, but who knows ? It's easier to relax a restriction rather than apply it after-the-fact.

Answer 3

From Effective Java

At the first glance :

you can put your class final

public final class Database {

and also the strings like :

final String file = directory.toString();
final String extension = file.substring(file.lastIndexOf("."));

Remove initialisation of fields :

private String dbName;
private SqlJetDb db;

Then in public void GetDirectoryContent use Files and Paths from new nio.2 java 7 package

And take care :

finally { // many possible problems

Read deeply and read again Joshua Bloch for good code, ... and enjoy yourself.

Answer 4

First of all, I'd leave the SqlJetException unchecked and not catch it in close method. I hate exceptions in close methods because what am I supposed to do with it? If I could do anything, I've already caught it, but most times it has to be muted catch(xxxx) {}.

I never use printStackTrace.

Does the GetDirectoryContent method insert records? The name is not clear enough, it's supposed to get a directory content, not insert any content. And it returns nothing... I think a better name would be something like

saveContentFrom(String directory)

Is this class a public API? If so, I would check for errors like "this param is null".

CreateDatabase, OpenDatabaseConnection and CloseDatabaseConnection are public, and you use them in GetDirectoryContent... what would happen if someone creates a database and calls your GetDirectoryContent? I would make those 3 methods private.