1
\$\begingroup\$

I am learning spring and am wondering about this service class's method. Is there a way to split this method up? Is there a way to use spring framework to make this method more readable? Should I move any of it to the controller or the DAO or should I split it into more service methods?

// UserBean has fields: username, password, first name, last name, etc.
// The UserBean parameter has 2 non-null fields: username and password.
public UserBean getUserDetails(UserBean bean) {
    UserEntity entity = new UserEntity();
    BeanUtils.copyProperties(bean, entity);

    // Gets user from DB based on UserBean's username and password.
    // Returns null if not found.
    entity = this.userDao.getUser(entity);

    if (entity == null) {
        return null;
    }

    // Fill in the rest of the UserBean's data from the DB
    UserBean userBean = new UserBean();
    BeanUtils.copyProperties(entity, userBean);
    // calculate time since last login based on DB timestamp
    userBean.setTimeSinceLastLogin(this.getSecondsSinceLastLogin(entity.getLastLogin()));

    // update DB with new timestamp
    entity.setLastLogin(new Date());
    this.userDao.updateUser(entity);

    return userBean;
}
\$\endgroup\$
  • \$\begingroup\$ Are you shure that you need to use copyProperties twice or at least at once? As fo me It can be ommited. \$\endgroup\$ – outoftime Oct 1 '14 at 5:48
  • \$\begingroup\$ How do you omit it? \$\endgroup\$ – rys Oct 1 '14 at 12:38
1
\$\begingroup\$

Perhaps if this method:

this.userDao.getUser(entity);

Accepted just a String as input (the username?) and possibly a password, you could then call it without having to mess around creating the UserEntity beans.

So this:

UserEntity entity = new UserEntity();
BeanUtils.copyProperties(bean, entity);

// Gets user from DB based on UserBean's username and password.
// Returns null if not found.
entity = this.userDao.getUser(entity);

Would become

UserEntity entity = this.userDao.getUser(bean.getUserName());

Maybe rename the entity/bean variable too to some thing that makes it a bit clearer what they are.

Also in the second half of the method, there seems to be no need to copy the properties to a UserBean and then back again. Just update the time since last login on the UserEntity directly.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Can you add code for the last paragraph? Not following it exactly. \$\endgroup\$ – rys Oct 1 '14 at 12:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.