Design/Code Problem - From Apress's book on Spring

Question

I was reading Apress "Beginning Spring 2 From Novice to Professional", and I found this one section that discusses usage of DAOs. The example is pretty simple and I have pasted the src here:

• UserAcccount.java - UserAccount Entity
• UserRole.java UserRole Entity
• UserAccountDao.java - UserAccount DAO
• JdbcUserAccountDaoImpl.java - JDBC Implementation of the UserAccount DAO

Advanced java/spring users can directly refer to the last link (jdbc implementation of the DAO). What interests me is this piece of the source code:

public List<UserAccount> list() {
      final List<UserAccount> list =
         getTemplate().query(SELECT_ACCOUNTS, userMapper);
      for(final UserAccount account : list) {
         populateRoles(account);
      }
      return list;
   }

This method is supposed to retrieve a list of all the UserAccounts in the database. The first list obtained by executing the query "SELECT_ACCOUNTS" only gets you the UserAccount details from the UserAccount table. The UserRoles however are on a different table. From the above piece of code, it is quite clear that for every UserAccount, populateRoles is executed. populateRules(account) in turn queries the roles table with the user id. Effectively, if you have 1000 users, this code would fire out a prepared statement 1,000 times!!

So here come the questions:

Section 1:

Isn't that a bad practice? Is there an easy way to this? How would you do it normally? What is the best-practice here?

Section 2:

How do you decide on DAOs? Should each entity have a DAO? For instance, in the above example, UserAccount and UserRoles represent two entities. On the database front, you have USERS, ROLES tables for the same and a USERS_ROLES mapping table. So, how would you decide on what DAOs to create? Does it depend on requirements? Or should I create a DAO when objects are related (have a mapping table in the database?)?

Answer 1

I would use lazy initialization in UserAccount#getRoles:

public Set<UserRole> getRoles() {
  if (roles == null) {
    XXX.populateRoles(this); // replace XXX with the class who populates the Roles.
  }
  return roles;

}

That way, only the UserAccounts who ever get asked about their roles, would actually ask the DB for them. I know, that can't work well if you have a code that asks them all for their roles, but the question there is: do you absolutely need the Roles of every UserAccount at startup?

Answer 2

section1 - yes, this is no so good practice because at this step you know all accounts and can load all roles with one sql select. But as this book is about Spring, and not about SQL, it's good enought. Normally, if you using ORM's like a Hibernate, it will optimize this for you, using lazy or bulk loading, for ex. But you need add some hints for it, manually configure hibernate for optimal performance, etc.

section2 - if DAOs are simple CRUD, it's better to make individual DAO for each entity. If you have some functional domain, you can split you DAOs by domain. And so on. Look at what for you will use your DAO, and what operations seems to be popular, and then group them at one DAO.

Answer 3

I think the Spring app is written to show you how different things can be done without worrying too much about performance.

Going by that logic ideally you should never be doing a list() or queryAll() especially on a user table (or for that matter most tables with > 1000s of rows). So yes, it is suboptimal and if ORM/Hibernate was not an option I would do it likes this.

//just queries userAccount with no roles populated
public List<UserAccount> listAllUsersRolesNotPopulated();

//get userAccount with roles for an accId
public UserAccount listUserWithRoles(int accId);

and the caller decides to call the second API when he wants roles.