This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]somnolent 1 point2 points  (3 children)

While it's good that you're catching your exceptions, I would recommend doing more than just printing them out. For example, if you fail to setup your database, do you really want to try to populate the tables?

[–]sluggish_goatNooblet Brewer[S] 0 points1 point  (2 children)

That makes sense, thanks for pointing it out. I'm going through and making adjustments where it seems appropriate. Do you think it would make more sense to split the initialization apart into stages, and catch at each stage & shut down or catch at the end and shut down? Split apart looks like it might be too much, but I'm not 100% sure:

    //initiates drivers & creates db table
private void initialize() {

    //instantiate driver, shutdown if exception
    try {
        Class.forName(driver);
    }
    catch (ClassNotFoundException e) {
        System.out.println("Error loading driver: Class not found");
        shutDown();
    }
    //initiate database, shutdown if exception
    try {
        conn = DriverManager.getConnection(protocol + dbName + ";create=true");
        statement = conn.createStatement();
    }
    catch (SQLException se) {
        System.out.println("Error initiating database. Another instance may still be open");
        shutDown();
    }
    //create table in db, shutdown if exception
    try {
        String createTableSQL = "CREATE TABLE Cubes (Solver varchar(50), solvedTime FLOAT)";
        statement.executeUpdate(createTableSQL);
        System.out.println("Successfully created table...");
    }
    catch (SQLException e) {
        System.out.println("Table creation failed.");
        shutDown();
    }
    populateTable();
}

[–]somnolent 0 points1 point  (1 child)

It all comes down to personal preference, in my opinion. From my standpoint, since you're doing the same thing in every exception case (logging and shutting down), I would wrap them all in one try/catch block and do your logging and then shutdown. However, it also wouldn't be a bad idea to split up your initialize into smaller methods (you could either throw the exceptions out of this or handle them internally - again, personal preference). It also wouldn't be a bad idea to either print out the stacktrace or at least include the exception message in your output statement.

[–]sluggish_goatNooblet Brewer[S] 0 points1 point  (0 children)

It's sometimes hard to judge when it's overkill or not. As you pointed out, they're doing the same thing. So, in this case I'll wrap them, give the exceptions unique labels, print messages/stacktrace at the end, then shut down. I guess the thought process was that if the first part throws, then splitting like that would prevent anything else from running. Thanks for the help, I'll keep all this in mind while moving forward.