awferreira / c5-db-migration

Automatically exported from code.google.com/p/c5-db-migration
0 stars 0 forks source link

Code review: com.carbonfive.db.jdbc.ScriptRunner #33

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
com.carbonfive.db.jdbc.ScriptRunner is a weird beast -- it requires a
dbType, but then always delegates to a new instance of ScriptRunnerImpl
(whose names also don't really follow the naming convention of an
SomethingImpl implementing Something).

It seems that ScriptRunner could be deleted if it remains in it's current
form, as long as you promoted ScriptRunnerImpl to be public.

Original issue reported on code.google.com by matthew.mceachen on 26 Feb 2010 at 9:04

GoogleCodeExporter commented 8 years ago
Matthew,

You're totally right.  My intention is to write db-specific script runners, but 
I just 
haven't gotten around to it.  So, what you're seeing is the harness for 
handling it.

It's been that way for a while and I haven't gotten around to writing any 
db-specific 
runners, though I've started working on a Postgres implementation.  It just 
hasn't 
seen any love in a while (and who knows when it will again).

The issue is that different DBs handle proc/func declarations differently.  I 
was once 
of the mindset that instead of a adding complexity to ScriptRunnerImpl for all 
of the 
cases, I would keep the implementations simple and specific to one DB.

Anyway, thanks for the nudge... I should either implement the next one or get 
rid of 
the extra layer. -Christian

Original comment by christia...@gmail.com on 26 Feb 2010 at 9:17

GoogleCodeExporter commented 8 years ago
Cleaned up.

Original comment by christia...@gmail.com on 24 May 2010 at 9:46