家银通项目bad smell 代码示例
?
/** * 获得远程EJB * @param <T> * @param T * @param url * @param location 1=账务核心 * 2=电视应用 * @return */ protected <T>T getRemoteService(Class T,String url,int location) { T service = null; InitialContext ctx = null; try { if(1==location) { ctx = getAccountCTX(); } if(2==location) { ctx = getApplyCTX(); } service = (T)ctx.lookup(url);} catch (NamingException e) {e.printStackTrace();}finally{if(null!=ctx){try {ctx.close();} catch (NamingException e) {e.printStackTrace();}}}return service; } private InitialContext getApplyCTX() { InitialContext ctx = null; try { Properties props = new Properties(); props.setProperty(Context.INITIAL_CONTEXT_FACTORY,"org.jboss.naming.NamingContextFactory"); props.setProperty(Context.PROVIDER_URL,PropertyManager.getProperty("EJB_TVAPPLY_URL")); props.setProperty(Context.URL_PKG_PREFIXES,"org.jboss.naming:org.jnp.interfaces"); ctx = new InitialContext(props);} catch (NamingException e) {e.printStackTrace();}return ctx; }?评点：自定义int型location不够直观，甚至连定义者也不见得每次都记得哪个对应哪个。可定义一个如下的静态变量。
public static final int LOCATION_ACCOUNTCTX= 1;
?但其实location这一变量用来对应不同的PROVIDER_URL，如下图：
props.setProperty(Context.PROVIDER_URL,PropertyManager.getProperty("EJB_TVACCOUNT_URL"));故完全可用相应的providerURL来代替location，且省去了if...else的判断语句。
优化：
?
public static final String TVAPPLY_PROVIDER_URL = PropertyManager.getProperty("EJB_TVAPPLY_URL"); protected <T>T getRemoteService(Class T,String url,String providerURL) { T service = null; InitialContext ctx = null; try { ctx = getCTX(providerURL); service = (T)ctx.lookup(url);} catch (NamingException e) {e.printStackTrace();}finally{if(null!=ctx){try {ctx.close();} catch (NamingException e) {e.printStackTrace();}}}return service; } private InitialContext getCTX(String providerURL) { InitialContext ctx = null; try { Properties props = new Properties(); props.setProperty(Context.INITIAL_CONTEXT_FACTORY,"org.jboss.naming.NamingContextFactory"); props.setProperty(Context.PROVIDER_URL,providerURL); props.setProperty(Context.URL_PKG_PREFIXES,"org.jboss.naming:org.jnp.interfaces"); ctx = new InitialContext(props);} catch (NamingException e) {e.printStackTrace();}return ctx; }??
/** * 说明 ： 获取最近登陆账户信息 * @param : 1. List ls, * * @return : Accountinfo */ @TransactionAttribute(TransactionAttributeType.MANDATORY) public Accountinfo getRecentAccount(List<String> ls) { ... Accountinfo accountinfo = (Accountinfo)query.getSingleResult(); return accountinfo; }?评点： 不够简洁且高并发情况下，还可能会发生“张冠李戴”的情况。
? 可优化为：
return (Accountinfo)query.getSingleResult();?
public List<AccountDTO> queryAccounts(String accountNo, String chanelCode, String registTimeStart,String registTimeEnd, String status, List<String> lsAccountInfoId, String merchantName,String radiocode,int startRow,int endRow){......}?
?评点： 参数过多，可用AccountDTO来代替。可能需要在该DTO增加几个属性来对应页面表单参数，如起始时间，结束时间及多选checkbox状态等。如下图：
?
?
/** * 说明 ： 账户账号密码登陆验证 * @param : 1. Long accountInfoId, * 2. int status * @return : long */@TransactionAttribute(TransactionAttributeType.MANDATORY)public long loginValidate(String accountNo, String loginPwd,String chanel){long returnValue=1; //失败//查询该帐户编号是否存在 String sql = "select count(*) from useraccnum u,accountinfo t where t.ACCOUNTSTATE!=4 and u.accountinfoid = t.accountinfoid and u.useraccount=? and u.channelinfoid=?"; Query query = em.createNativeQuery(sql); query.setParameter(1, accountNo); query.setParameter(2, chanel); BigDecimal b = (BigDecimal)query.getSingleResult(); //----如果验证成功则取得账户编号 if(null!=b&&1==b.intValue()) { //取得账户ID sql = "select u.accountinfoid from useraccnum u where u.useraccount=? and u.channelinfoid=?"; query = em.createNativeQuery(sql); query.setParameter(1, accountNo); query.setParameter(2, chanel); b = (BigDecimal)query.getSingleResult(); returnValue = b.longValue(); //账户流水 //验证密码 sql = "select loginpwd from useraccnum u,accountinfo t where t.ACCOUNTSTATE!=4 and u.accountinfoid = t.accountinfoid and u.useraccount=? and u.channelinfoid=?"; Query querymm = em.createNativeQuery(sql); querymm.setParameter(1, accountNo); querymm.setParameter(2, chanel); String dPwd = (String)querymm.getSingleResult(); //验证不通过 //if(!PasswordUtil.validate(dPwd, loginPwd)) if(!dPwd.equals(loginPwd)) { return -returnValue; } //-----更新登陆时间 sql = "update useraccnum t set t.recentlandtime=sysdate where t.useraccount=? and t.channelinfoid=?" ; query = em.createNativeQuery(sql); query.setParameter(1, accountNo); query.setParameter(2, chanel); query.executeUpdate(); } return returnValue;}?
评点：该方法用于账户登录时的校验。功能过于复杂、凌乱。先是查询记录数，若为1，则再查询主键ID,接着查询其密码（以上三步仅查询一次便可解决），校验密码是否匹配，更新登录时间。且返回值也让人眼花缭乱，账户不存在返回1，账户存在密码错误则为主键ID的负值，若校验通过则为主键ID。
优化：功能单一化，仅校验登录信息。登录日志（登陆成功、登录失败）另外处理。返回值为boolean，即告知校验通过还是未通过。
/** * 1. 根据账户ID进行账户锁定 * * @param 1. * 账户ID Long accountInfoId 2. 锁定方式 int lockType 0：自动锁定；1：手工锁定； * @return OutObj4Account * @exception */ public OutObj4Account lockAccount(Long accountInfoId, int lockType) { OutObj4Account outObj4Account = new OutObj4Account(); try { ut.begin(); if (null != accountInfoId) { if (0 == lockType) { accountDAO.updateStatusById(accountInfoId, 0); } if (1 == lockType) { accountDAO.updateStatusById(accountInfoId, 1); } outObj4Account.setIsSuccess("0"); outObj4Account.setMsg("操作成功"); ...... }??
评点： 多此一举。
优化：
accountDAO.updateStatusById(accountInfoId, lockType)?
/** * 1. 根据账户ID进行账户解锁 * * @param 1. * 账户ID Long accountInfoId 2. 解锁方式 int unLockType 0：自动解锁；1：手工解锁； * @return OutObj4Account * @exception */ public OutObj4Account unlockAccount(Long accountInfoId, int unLockType) { OutObj4Account outObj4Account = new OutObj4Account(); // ------------------------------------------------------- try { ut.begin(); if (null != accountInfoId) { accountDAO.updateStatusById(accountInfoId, 3); outObj4Account.setIsSuccess("0"); outObj4Account.setMsg("账户解锁操作成功"); } else { ...... } ...... }?
评点： unLockType参数多余。且校验accountInfoId亦属画蛇添足。很明显在该操作前你已明确知道该主键ID。再说也不能单单校验是否为null，还得判断是否为空，是否合法，是否存在等。