家银通项目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，还得判断是否为空，是否合法，是否存在等。