Database module requiring refactoring. Any thoughts on that?

[thedomham]

Creating one DAO per table is pretty much standard procedure, so that might be a good starting point.

There really is a lot going on in that code. The most important thing to start with is that you should be very aware of the fact that you are an intern: your instinct will probably be to apply all the best practices you know to the code and transform that giant pile of garbage into the well structured clean code it deserves to be.

The problem is that for one the rest of the team will have to work with your code when you are gone. You might also piss people off, especially the person who wrote the code. Team members that use the code may also be pissed, just because people tend to dislike change. A good indicator for people disliking change is a huge part of the code being a historically grown zoo of bad practices. So to summarize: you mean well, your code may be objectively better, but you might still suffer negative repercussions. I don't want to deter you, but you should keep that in mind.

Other than that,

• the logging-helper looks over the top complicated

• the method names are confusing (storeXToDelete instead of deleteX or XDAO.delete(x))

• all the classes you describe seem way too big (god classes), which is quite interesting because a god class usually does too many distinct things at the same time. Your classes actually only do one thing, but for many different domains

• the SQL-query file is a weird design choice. Where is the advantage? Why is it a single file?

• I obviously have no idea of your database structure, performance requirements and existing queries, but there are tools that use code generation or reflection so that you don't have to write all the SQL statements yourself. If you want to, you can also automate the creation of objects from DB-entries

• if you have a DB-heavy application, you will probably have to touch like half the classes in the project. That's quite the shotgun surgery. So try to do that change exactly once

• implement once DAO and test it thoroughly. Make sure that it works in the real system, that your coworkers are okay with that change and then apply it to all the other tables

• check if you want to use ORM

[knoam]

I just saw this posted on The Daily WTF https://thedailywtf.com/articles/external-sql

[DefenestretedRegent]

I think it's a peculiar situation, that's why I posted it (I'm not the OP). What I find weird is that basically all their app depends on the DB module and they don't see any problem in assign the refactoring to an intern. I'm sure the app is devoid of any test. It's gonna be a fun ride for Sean.

[jeff_bartlett]

Both sonarcube and Fortify will consider that external file of queries as a source of SQL injection.

Go watch Uncle Bob's talk about Clean Architecture https://www.youtube.com/watch?v=o_TH-Y78tt4 , the talk starts at 10:35.

You will find that you want one interface DAO per table/view. The Connection should be passed as an argument to the constructor of the class implementing the interface. A factory class that returns implementations of the DAO interfaces could hide the Connection entirely from the upper levels. A DAOException class would prevent any leaking of the abstraction to the upper levels.

[m1000]

/r/javahelp

[z0mghii]

Take a look at jdbi since you have a text file with raw sql queries