HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Constructing NewsArticle objects from a database table

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
objectsdatabaseconstructingnewsarticlefromtable

Problem

This class seems to be able to represent the data stored in any possible database table:

public class DbTable {
private Set> tblData = new HashSet>();

public DbTable(Connection dbConn, String tblName) throws Exception {
PreparedStatement prpStmt = dbConn.prepareStatement("SELECT * FROM " + tblName);
ResultSet queryResults = prpStmt.executeQuery();
ResultSetMetaData metaData = queryResults.getMetaData();

while (queryResults.next()) {
Map row = new HashMap();

for (int i = 1; i

Then, I'd like to construct objects to represent news articles based on the
Map retrieved via DbTable, like this:

public class NewsArticle {
private final String Id, Title, Story;
private final Timestamp Pubdate;

public NewsArticle(Map row) {
String id = null; String title = null; String story = null;
Timestamp pubdate = null;

for (Map.Entry col : row.entrySet()) {
if (col.getKey().equals("id")) {
id = (String) col.getValue();
continue;
}
if (col.getKey().equals("pubdate")) {
pubdate = (Timestamp) col.getValue();
continue;
}
if (col.getKey().equals("title")) {
title = (String) col.getValue();
continue;
}
if (col.getKey().equals("story")) {
story = (String) col.getValue();
}
}

Id = id;
Pubdate = pubdate;
Title = title;
Story = story;
}

// ...
}


So, a
Map represents a database table entry as the column name, mapEntry.getKey(), and the data in the column, mapEntry.getValue().

Because I lose type safety, I'm thinking this is rubbish. But, (1) users of
DbTable most certainly should know the type of each column. (2) if they do not know the type, then they can use reflection as a backup way to make DbTable` usable.

This

Solution

Coding Issues

-
In NewsArticle, the names of the attributes begin with uppercase letters (Id, Title, Story, Pubdate). This violates common Java field naming conventions.

-
Database column names are usually case insensitive. Depending on the underlying DBMS, the instructions like col.getKey().equals("id") may fail, because the searched value could have been stored under e.g. ID key when being read in DbTable. Either it should be normalized by applying toLowerCase() there, or it should be checked with equalsIgnoreCase() in NewsArticle constructor.

-
Looping over the entire row map in NewsArticle constructor is unnecessary. The fields may be initialized with e.g. id = (String) row.get("id"); and there is no need to use local variables id, title etc.

-
In DbTable: Class colClass var is not typed (raw type), which is not good. This variable is useless, because colValue is an Object and can be directly initialized with queryResults.getObject(arg), without casting.

Design Issues: Critical!

There are some very important issues concerning DbTable class.

-
Loading the contents of an entire table using tblData Set is very dangerous. First of all, if the table is rather huge (imagine just several dozens of thousands of rows), this object will squatter the memory and probably consume too much of it uselessly. It also duplicates the data supposed to be stored in the DB and produces too much overhead on each DbTable object creation by reading all the data. However, reading just the metadata of table columns and their types could have been useful for your problem.

-
prpStmt and queryResults objects are created, but never closed. Initializing them inside a try-with-resources block will solve the issue.

-
throws Exception, especially in a constructor, looks very ugly. Either replace it with a more concrete exception type or (even better) find a way to avoid the throws clause for the constructor.

-
There are no validity checks for Connection and String arguments. What will happen, for example, if one of them or both are null?

These remarks do not answer your initial question about the type safety of the solution. It's too early to answer it before fixing many of the critical issues.

Context

StackExchange Code Review Q#108796, answer score: 2

Revisions (0)

No revisions yet.