NOTE: I was given permission to post the codes here.

I am in my internship currently. We are maintaining a system that is quite used in the organization. It is written in Java. I am assigned to refactoring the code and I chose to work on the database module, thinking it was just an easy job to do.

When I saw the code, it is full of singletons and I feel that the module could be done in a better way. One of the classes in the module is InsertDatabase . It handles every data insertion. It looks like this:



public class InsertDatabase { private static InsertDatabase instance = null ; private Properties prop = new Properties (); private InputStream input = null ; private InsertDatabase (){ try { input = new FileInputStream ( "textfile/sql_queries.txt" ); prop . load ( input ); } catch ( Exception e ){ System . err . println ( e ); } } public static InsertDatabase getInstance (){ if ( instance == null ) { instance = new InsertDatabase (); } return instance ; } public void insertIntoCourseTable ( String course_abbr , String course_name ) { Connection conn = cpInstance . getConnection (); try ( PreparedStatement ps = conn . prepareStatement ( prop . getProperty ( "insert_course_table" ));) { ps . setString ( 1 , course_abbr ); ps . setString ( 2 , course_name ); ps . executeUpdate (); Helper . getInstance (). aprintln ( this , Helper . LOG_VERBOSE , "" ); Helper . getInstance (). aprintln ( this , Helper . LOG_VERBOSE , "*** Information Added into Course Table***" ); } catch ( SQLException e ) { Helper . getInstance (). aprintln ( this , Helper . LOG_VERBOSE , "Data already exists" ); } finally { try { conn . close (); } catch ( SQLException e ) { e . printStackTrace (); } } // more code... }

This is also the same case with the other classes assigned to table creation, updating data, and selecting data.

The SQL queries are actually stored in a text file called sql_queries.txt and is loaded in the variable props . The text file is almost 200 lines long. Here's a sample line from the text file.



insert_diagnostic_table = insert into diagnostic_table (date_start, date_end, sem_num, freetime, time_penalty, balance_penalty) values (?, ?, ?, ?, ?, ?) insert_pasa_balance_table = insert into pasa_balance_table (date_rendered, sender, amount, current_balance_sender, deducted_balance_sender, receiver, current_balance_receiver, added_balance_receiver) values (curdate(), ?, ?, ?, ?, ?, ?, ?)

The classes are then herded into a singleton class called Information , which the system uses to interact with the database. The class looks like this:



public class Information { private static Information instance = null ; private InsertDatabase insert_db = InsertDatabase . getInstance (); private SearchDatabase search_db = SearchDatabase . getInstance (); private UpdateDatabase update_db = UpdateDatabase . getInstance (); private DeleteDatabase delete_db = DeleteDatabase . getInstance (); private TransDatabase trans_db = TransDatabase . getInstance (); public static Information getInstance (){ if ( instance == null ) { instance = new Information (); } return instance ; } /* * Here lies all of the methods that access DeleteDatabase */ public void storeToDeleteReservation ( String username ) { delete_db . deleteReservation ( username ); } // more code... public void storeStudentToDelete ( String username ) { delete_db . deleteStudent ( username ); } /** * Deletes Staff * @param username */ public void storeStaffToDelete ( String username ) { delete_db . deleteStaff ( username ); } /* * End of methods that access DeleteDatabase */ /* * Here lies all the methods that access InsertDatabase */ public void storeAccountInfo ( String username , String password , String last_name , String first_name , String mid_name , String suffix_name ){ insert_db . insertIntoAccountTable ( username , password , last_name , first_name , mid_name , suffix_name ); } public void storeCourseInfo ( String course_abbr , String course_name ) { insert_db . insertIntoCourseTable ( course_abbr , course_name ); } // more code... }

I have a weird feeling that there are problems with this. I can't pinpoint exactly what the problems are. Having them as singletons is a warning flag for me already. So, are there any problems with the approach?

What I was thinking to do was to create a DAO (data access object) for each table and have the rest of the system use the DAOs instead. If I proceed with DAOs, I'd modify Information first so that I would not touch the rest of the system and accidentally break anything. If I do break anything, at least I know where I screwed up. What do you think?

Thanks! :)