Minor detail: I prefer putting a closing parenthesis on its own line when the content between parentheses spans more than one line. (Basically, it looks like one-true-brace style with parentheses instead of braces.) I find it makes scanning vertically much simpler. If your team/company has chosen a different convention, that's fine.
The biggest problem I see with your code is that there's no division of responsibilities. For a very simple program, this doesn't matter, but in more complex application with a bunch of different queries and behaviors, it's a huge deal. The overhead of doing it isn't that big, either, so for almost any real world program, I recommend going ahead and dealing with this. (There's nothing more permanent than a temporary, quick, and dirty solution, after all.)
Creating the connection
Start by creating the connection in one place. This lets you reuse that code in multiple places in your application, and we'll see in a minute that it makes it much simpler to execute multiple queries in a transaction.
class OraConnection {
public static Connection openConnection(
String db_host_name,
String db_port,
String db_service_name,
String db_username,
String db_password
) {
String connection_string = String.format(
"%s:%s:%s:@%s:%s/%s",
"jdbc",
"oracle",
"thin",
db_host_name,
db_port,
db_service_name
);
Class.forName("oracle.jdbc.driver.OracleDriver");
return DriverManager.getConnection(
connection_string,
db_username,
db_password
);
}
}
I also recommend hard coding the "jdbc", "oracle", "thin" string here. The main reason is that the way you're opening the connection doesn't support any other values for them. For example, you can't open a PostgreSQL connection with this code because it looks for the oracle.jdbc.driver.OracleDriver class. Since the caller can't validly pass in anything else, there's no point in providing them as arguments. If your connection opening code were more general, I wouldn't advise that. However, this exposes one of the advantages of breaking things down into smaller pieces: you could change this code to be more general and allow more connection types without changing any of your other code.
Fetching results
Then create an object to represent your query results. Callers that want to execute your query and process the results shouldn't have to worry about details like a result set, what position each column is at, and what data type each column is. All that should be sorted out by the method that executes the query, and that method should return a simple object containing the results.
class SomeTable {
private int column1;
private int column2;
private String column3;
// ...and the rest of your columns
public SomeTable(
int column1,
int column2,
String column3,
// the rest of your cols
) {
this.column1 = column1;
// etc.
}
public int getColumn1() { return this.column1; }
// the rest of your columns
}
Then put the query itself in another class.
class SomeTableQueries {
public static List<SomeTable> fetchAllSomeTableRows(Connection connection) {
String query = "select column1, column2, column3, /* rest of your columns */ from some_table";
try (
Statement statement = connection.createStatement();
ResultSet result = statement.executeQuery(query)
) {
ArrayList<SomeTable> rows = new ArrayList<SomeTable>();
while (result.next()) {
rows.add(SomeTable(
result.getInt(1),
result.getString(2),
result.getString(3)),
// rest of your columns
));
}
return rows;
}
}
}
Note that it does the following:
- It hides the boiler plate of building a statement and converting the data into a simpler form from callers.
- It avoids
SELECT *. SELECT * is okay if you have code that can automatically figure out what the result columns are and stuff them into an appropriate object, but we haven't written code like that here. Since our column set and the order is hard coded as part of reading the data, it's better to list them explicitly.
- It cleans up the
Statement and ResultSet objects it creates.
- Names the method to describe the query it runs
Logic that uses all this
Now that we've got separate pieces of code to do smaller pieces, we can write our main method. Note that this method must be responsible for ensuring that the connection gets closed properly.
public class PrintSomeTable {
public static void main(String[] args) {
try (Connection conn = OraConnection.openConnection(
"host",
"port",
"service-name",
"user",
"pass"
)) {
for (SomeTable r: SomeTableQueries.fetchAllSomeTableRows(conn)) {
System.out.println(
r.getColumn1() +
" " +
r.getColumn2() +
" " +
r.getColumn3()
);
}
}
}
}
Note that it would be very simple to use the same connection for more queries and operations.
Closing notes
The overall theme here is that you should use methods and classes to hide complexities that callers don't really care about, which makes understanding the code much simpler. This way, you can consider the details of each operation in an isolated scope, instead of trying to understand what each "larger" logical operation is by sorting through every little detail.
Lastly, if you have a lot of queries, seriously consider using an ORM, a micro ORM, or other similar tooling to handle a good amount of this boiler plate for you.
