Is this good approach to design DB i/f?

[Manohar Karamballi]

Hi,
I am following below mentioned approach for handling database transactions.
-- A class that have all static methods such as autheticateUser , getResults etc..(one method for each db transaction)..
-- Class has static initilizer that assign static datasource by JNDI lookup
-- Each time method is invoked Connection object is retrieved from static DataSource, database related things are done and closing connection.
-- If any exceptions are thrown I am closing connection and returning null from static method.
I need feedback regarding following points.
1. Is Initializing DataSource (and making it as static ) have ne side effects?
2. Is static method approach recommended?
3. Any performance penalty on getting connection from DataSource each time?
4. What is best way of handling exceptions? Is it good practice to return null object?
5. I am in notion that closing connection close all child objects such as Statements and Resultsets. Is this correct?
6. And ne comments regarding other drawbacks of proposed design and enhancements that could be done to this design...
My DBConnection i/f should support peak load of around 50 requests.
Aniticipating your valuable suggestions.
Thanks in advance.

[Frank Carver]

You have told us about your proposed design, but you have not told us much about the reasons you chose it. I hope you have some pretty strong reasons for using this approach; In my experience this would be a very hard design to test, debug and maintain.
1. Is Initializing DataSource (and making it as static ) have ne side effects?
2. Is static method approach recommended?
Using public static methods forces you to compile-in the name of your "db access" class to every class which uses it. This makes it very hard to test or debug the rest of the code without a live database connection available - you can't substitute a "mock" database access class which simply returns known values for testing.
Using static initialisers is very dangerous. The JVM specification does not have much to say about the order in which initialisers are run, so you can't assume (for example) that your JNDI access classes have been initialised properly before your static initialiser runs. My rule of thumb for initialisers is to only ever use them for default values for primitives or simple objects such as Strings. Everything else should be done in a constructor or as needed.
Also, static initialisers are run at class-load time, so if your db access class is anywhere in your class path, every Java program you run will try and open a JNDI connection and retrieve a DataSource whenever it starts. I find it hard to believe you actually want this strange behaviour.
3. Any performance penalty on getting connection from DataSource each time?
This depends on your DataSource. If you use a DataSource implementation which pools connections, there will be very little overhead. If you use a naive implementation which actually closes connections, you could incur a large database performance hit.
4. What is best way of handling exceptions? Is it good practice to return null object?
This entirely depends on what your clicnt code does with the returned value. In some situations it might be better to return a known "empty" object, or pass in a "default" object to be returned on error cases, or just pass on the exception. I can't really help you any more without knowing more about how this code will be used.
5. I am in notion that closing connection close all child objects such as Statements and Resultsets. Is this correct?
I wouldn't rely on this. It is a common approach in pooled DataSources to override the "close" method of Connection to just return the connection to the pool. It's much better practice to always close things when you have finished with them.
6. And ne comments regarding other drawbacks of proposed design and enhancements that could be done to this design...
Please help us by explaining why you feel that a static (non-Object-Oriented) solution has any benefits at all. The obvious, simple, solution would seem to be a regular class with a constructor which finds the DataSource, and other methods which use it. Can you give your reasons why did you not just choose this approach ?

[Manohar Karamballi]

Dear Frank,
First of all I would like to thanku for your detailed reply.
I am going to give reasons for some of the approches I followed. I might be wrong as I am relatively new to this OOA/D.
The first thing why I made my dbacess class to contain all static methods is that I don't really need to have an instance of DBAcess class. I invoke methods like DBAcess.authenticateUser()and hence I need not to instantiate class again and again (Whenever required.).
Using public static methods forces you to compile-in the name of your "db access" class to every class which uses it. This makes it very hard to test or debug the rest of the code without a live database connection available - you can't substitute a "mock" database access class which simply returns known values for testing.
At any point I can change implementation of methods so that they return mock values instead of live DBConnection even in static method approach.
Using static initialisers is very dangerous. The JVM specification does not have much to say about the order in which initialisers are run, so you can't assume (for example) that your JNDI access classes have been initialised properly before your static initialiser runs. My rule of thumb for initialisers is to only ever use them for default values for primitives or simple objects such as Strings. Everything else should be done in a constructor or as needed.
This problem is one which i donno before. So one way of overcoming this problem is to make class singleton and initialize Datasource in constructor. Correct me If I am wrong.
This depends on your DataSource. If you use a DataSource implementation which pools connections, there will be very little overhead. If you use a naive implementation which actually closes connections, you could incur a large database performance hit
My DBSource pools connections. I asked question to make sure that even with pooling we don't have ne problems.
--
From above discussion I understood that assigning Datasource in Static Initializer will have serious consequences. So I modify design as follows.
Instead of assigning data sources in static initializer I assign them in Constructor and make my class as Singleton.
I persist with Static method approach. (Pls provide drawback if ne)
Thanks once again.
Regards
Manohar

[Manohar Karamballi]

?

[Frank Carver]

I am relatively new to this OOA/D.

The first thing why I made my dbacess class to contain all static methods is that I don't really need to have an instance of DBAcess class. I invoke methods like DBAcess.authenticateUser()and hence I need not to instantiate class again and again (Whenever required.).
I certainly wasn't suggesting that you "instantiate class again and again"! When I do this sort of thing I instantiate the DB Access class just once, and pass it in (either as a constructor parameter to be saved for later, or a parameter to the methods which use the database) whenever needed. Consider the following simplified code:

interface DBAccess
{
  public User login(String name, String pasword);
  public Table getResults(String query);
}
  
class OracleDBAccess implements DBAccess
{
  public OracleDBAccess(String dbHost, String dbSid)
  {
    ...
  }
  
  public User login(String name, String pasword)
  {
    ...
  }
  
  public Table getResults(String query)
  {
    ...
  }
}
  
class Dealership
{
  private DBAccess db;
  private Table stock;
  
  public Dealership(DBAccess db)
  {
    this.db = db;
  }
  
  public void load(String manufacturer)
  {
    stock = db.getResults("select model, price from catalog where manufacturer='" + manufacturer + "'");
  }
  
  public String getPrice(String model)
  {
    Row row = stock.findFirstRow("model", model);
    return row.getColumnAsString("price");
  }
}

You don't really need to know how the Table or Row classes work, just that this shows an example of passing in an already created DBAccess class for the "business logic" class to use. Note also that the "business logic" nether knows nor cares about the actual class of the DBAccess object it is using. That's what I mean when I say that using static methods "forces you to compile-in the name".
At any point I can change implementation of methods so that they return mock values instead of live DBConnection even in static method approach.
Wow. It's a long time since I've heard anyone suggest this sort of thing. Most people I know would consider this far too risky. All it takes is a little bit of distraction and you can inadvertently ship the "broken" version instead of the real one. What I am suggesting is more like:

class MockDBAccess implements DBAccess
{
  public User login(String name, String pasword)
  {
    return new DummyUser(name, password);
  }
  
  public Table getResults(String query)
  {
    return new ArrayTable(new String[][]
    {
      { "model", "price" },
      { "Arosa", "8995" },
      { "Leon", "14995" },
      { "Alhambra", "18995" },
    }};
  }  
}
  
class DealershipTest extends TestCase
{
  public void testGetPrice()
  {
    DBAccess db = new MockDBAccess();
    Dealership dealer = new Dealership(db);
  
    dealer.load("Seat");
    assertEquals("8995", dealer.getPrice("Arosa"));
    assertEquals("14995", dealer.getPrice("Leon"));
  }
}

Using this approach I have been able to test the behaviour of the Dealership class without modifying it in any way, without requiring expensive and cumbersome database setup or a connection to a live databas, and without modifying the real DBAccess class. Isn't that a cleaner and less risky way of doing things ?
So one way of overcoming this problem is to make class singleton and initialize Datasource in constructor. Correct me If I am wrong.
Instead of assigning data sources in static initializer I assign them in Constructor and make my class as Singleton.
There has been a lot of discussion of the merits of Singleton in this forum. Reading the results of a search for "Singleton" would probably give you much more than I could write in this reply.
Do you still feel that a static method approach is as useful as one using regular objects and methds ?

[Manohar Karamballi]

Hi Frank,
Thanks once again for ur time...
Do you still feel that a static method approach is as useful as one using regular objects and methds ?

No..I left this approach..Instead I am getting singleton Instance and then invoking method on that...
like DBAcess.getInstance().autheticateUser(userName,password)

Wow. It's a long time since I've heard anyone suggest this sort of thing. Most people I know would consider this far too risky. All it takes is a little bit of distraction and you can inadvertently ship the "broken" version instead of the real one. What I am suggesting is more like:

I agree with you regarding deployment problems but other than that there is no problem to test with mock class.

Than regarding DBAcess i/f I have some concerns.
Let's walk thru the code u have given..
; public Dealership(DBAccess db) { this.db = db; }

Now constructor is taking paramter some DB i/f which not really related to class Dealership. That means we are trying to assign extra responsibilities to our class which it is not supposed to do if u go through by characteristcs of class Dealership. Suppose when u say object Dealership the responsiblities can be getDealershipdetails... But querying part is not functionality that one can assign to Delaership class.

stock = db.getResults("select model, price from catalog where manufacturer='" + manufacturer + "'");
Here what we are trying is to tie up our business class Dealership with particular query mechanism.
i.e to say Query acess...Tommorrow If I need to chnage implementation to Stored procedures I have to change all my business classes...Or worst If i try to abstract DB acess mechanism to something like JDOs or Entity beans I have to change all the code.. Instead we can delegate this responsiblity (getting data..whether it is directly from DB or using JDO or Entity beans)to separate class and our Business class (Dealership)is completely independent of DB acess mechanism or for that matter persistance store mechanism....
Thanks
Manohar

[Frank Carver]

Than regarding DBAcess i/f I have some concerns. Let's walk thru the code u have given..
I hope I haven't confused you by giving a deliberately simplified example. May aim with this was solely to show that you can design a "business" class which uses some sort of data source without knowing which class it is. The actual code in the Dealership and OracleDBAccess classes are purely illustrative.
; public Dealership(DBAccess db) { this.db = db; }
Now constructor is taking paramter some DB i/f which not really related to class Dealership. That means we are trying to assign extra responsibilities to our class which it is not supposed to do if u go through by characteristcs of class Dealership.
No. quite the opposite. This is just saying that Dealership doesn't need to do the database access itself, and giving it an object to do the database access for it.
Suppose when u say object Dealership the responsiblities can be getDealershipdetails... But querying part is not functionality that one can assign to Delaership class.
If you don't want the Dealership class to do the querying itself, you can easily make another class to do that:

class Dealership
{
  ...
  
  private Table modelPrice;
  
  public setModelPriceTable(Table table)
  {
    this.modelPrice = table;
  }
}
  
class DealershipLoader
{
  private DBAccess db;
  
  public DealershipLoader(DBAccess db)
  {
    this.db = db;
  }
  
  public load(Dealership dealer, String manufacturer)
  {
    Table results = db.getResults("select model, price from catalog where manufacturer='" + manufacturer + "'");
    dealer.setModelPriceTable(results);    
  }
}

Tommorrow If I need to chnage implementation to Stored procedures I have to change all my business classes...Or worst If i try to abstract DB acess mechanism to something like JDOs or Entity beans I have to change all the code.. Instead we can delegate this responsiblity (getting data..whether it is directly from DB or using JDO or Entity beans)to separate class and our Business class (Dealership)is completely independent of DB acess mechanism or for that matter persistance store mechanism....
Sure. Does the small change I show above illustrate that ?
[ January 07, 2003: Message edited by: Frank Carver ]

[Manohar Karamballi]

Hi Frank,
Seems Interesting...
Now we delegated data access responsiblity to different object and that is what exactly what I am doing.
But now I have some points regarding different object that we are using for DBAcess. public load(Dealership dealer, String manufacturer) { Table results = db.getResults("select model, price from catalog where manufacturer='" + manufacturer + "'"); dealer.setModelPriceTable(results); }
Here we are passing refernce of Dealer to DBAcess object. I think this introduces some coupling between DBAcess object and Dealer object.
Suppose I need to use same Query (data) from some other object say Supplier then I need to pass Supplier reference to DBAccess class and code need to be repated. like
public load(Supplier supplier, String manufacturer) { Table results = db.getResults("select model, price from catalog where manufacturer='" + manufacturer + "'"); supplier.setModelPriceTable(results); }
Take general & simple scenario where I have single legacy system which is being used by many applications.
All applications have login module and need to authticate users before login using some DB acess mechanism.
so our code will be

public authenticate(App1 app1, userName, password)
{
//Access DB
app1.setLoginStatus(val)
}
public authenticate(App2 app2, userName, password)
{
//Access DB
app2.setLoginStatus(val)
}

Further it forces me to implement setLoginStatus method whereever I want to use authenticate user.
i.e indirectly we are implemeting some i/f like thing in our business classes.
To simplify this we can implement DBAcess object with no reference to invokers like
public authenticate( userName, password)

[Frank Carver]

Here we are passing refernce of Dealer to DBAcess object. I think this introduces some coupling between DBAcess object and Dealer object.
Well, yes. What I have done is split the responsibility into three parts: the "business object" which only knows about its data and what operations to perform on it, the "specific data access object" which knows how to fetch a particular type of business object from a generic data access object, and the "generic data access object" which knows nothing about business but can handle any database operation on behalf of another object.
Suppose I need to use same Query (data) from some other object say Supplier then I need to pass Supplier reference to DBAccess class and code need to be repated.
If you really need the same code, then I'd suggest that there is some sort of common class or interface trying to appear. Maybe something like:

class StockHolder
{
  protected Table stock;
  public void setStock(Table stock)
  {
    this.stock = stock;
  }
}
  
class Dealer extends StockHolder { ... }
class Supplier extends Stockholder { ... }
class StockHolderLoader
{
  // similar to DealerLoader from before.
}

Take general & simple scenario where I have single legacy system which is being used by many applications. All applications have login module and need to authticate users before login using some DB acess mechanism.
This is better. I hope we will be able to understand each others' examples much better if we focus on a real application.
so our code will be:
public authenticate(App1 app1, userName, password)
{
//Access DB
app1.setLoginStatus(val)
}
public authenticate(App2 app2, userName, password)
{
//Access DB
app2.setLoginStatus(val)
}
Can you explain a bit more what 'val' is, where it comes from, and what 'setLoginStatus' does?
This doesn't look much like the authentication code I'm used to, which usually involves some sort of 'User' object which is created or filled in during a login attempt.
Further it forces me to implement setLoginStatus method whereever I want to use authenticate user.
Presumably you could push that method call into whatever code you use inside your 'Access DB' comment, though ? Or is it not needed in all cases ?
To simplify this we can implement DBAcess object with no reference to invokers like
public authenticate( userName, password)
I'd expect to see a method like this on a common Application object, extended by all the actual Applications. And I'd xpect it to just delegate to the more general one:

class GenericApplication
{
  protected DBAccess db;
  
  public GenericApplication(DBAccess db)
  {
    this.db = db;
  }
  
  public authenticate(String name, String password)
  {
    db.authenticate(this, name, pasword)
  }
}
 
class AnActualApplication extends GenericApplication
{
  public AnApplcation(DBAcess db)
  {
    super(db);
  }
 ...
}

Or have I missed your point again ?
I'm slightly puzzled why the low-level authentication method is not something like:
public boolean isValidUser(String userName, String password)
If it were, you could leave it up to the application to decide what to do if the userName/password is or is not valid.
I hope you are still finding this useful or interesting. I know I am.