Filling name accounts

I think the content is too large and I am deeply think about how to refactor it. Does anyone have good advice on refactoring it?

List<AccountEntity> fillNameAccounts = agentDao.getAgentListByIds(accountIdList);
    for (T bean : results)
    {
        for (AccountEntity entity : fillNameAccounts)
        {
            String chineseName = entity.getChineseName();
            String firstName = entity.getFirstName();
            String fillName = StringUtils.isBlank(chineseName) ? firstName : chineseName;
            String entityId = entity.getId().toString();
            if (bean instanceof AgentPostResponseBean)
            {
                if (StringUtils.equals(((AgentPostResponseBean) bean).getParentId(), entityId))
                {
                    ((AgentPostResponseBean) bean).setParentName(fillName);
                }
                if (StringUtils.equals(((AgentPostResponseBean) bean).getPosterId(), entityId))
                {
                    ((AgentPostResponseBean) bean).setPosterName(fillName);
                }
            }
            else if (bean instanceof ImportAgentPostResponseBean)
            {

                if (StringUtils.equals(((ImportAgentPostResponseBean) bean).getParentId(), entityId))
                {
                    ((ImportAgentPostResponseBean) bean).setParentName(fillName);
                }
                if (StringUtils.equals(((ImportAgentPostResponseBean) bean).getPosterId(), entityId))
                {
                    ((ImportAgentPostResponseBean) bean).setPosterName(fillName);
                }
            }
        }
    }

In this case, you take an action depending on class type of bean, so I would recommend you to implement a Visitor Pattern, because it is not god practice to use instanceof and because Visitor Pattern lets you growth your code without coupling.

My answer is more complex than previous, but it will give you more flexibility. What if you want to do more things than setNameForEntity? or what if you wnat to implement more concrete classes? Visitor Pattern will suite this needs.

Now, here comes how I would do the refactor. First, create an abstract class (or an interface) as base class for beans, like AgentPostResponseBean and ImportAgentPostResponseBean. This base class have a method accept(Visitor visitor).

public abstract class AbstractResponseBean{    
    public abstract void accept(Visitor visitor, AccountEntity entity);
}

If your ResponseBeans already inherit from an existing abstract class or they implement an existing interface, simply add the accept method to that class/interface.

Then, implement visit method on each concrete class, as follows

public void accept(Visitor visitor, AccountEntity entity){
    visitor.visit(this, entity);
}

Finally, create a Visitor class, that accept the visit of each concrete ResponseBeans class you are interested in. For example:

public class Visitor{

    private String getFillName(AccountEntity entity){
        String chineseName = entity.getChineseName();
        String firstName = entity.getFirstName();
        return StringUtils.isBlank(chineseName) ? firstName : chineseName;
    }

    public void visit(AgentPostResponseBean respBean, AccountEntity entity){

        if (StringUtils.equals(respBean.getParentId(), entityId)) {
            respBean.setParentName(getFillName);
        }
        if (StringUtils.equals(respBean.getPosterId(), entityId)) {
            respBean.setPosterName(getFillName);
        }
    }

    public void visit(ImportAgentPostResponseBean respBean, AccountEntity entity){

        if (StringUtils.equals(respBean.getParentId(), entityId)) {
            respBean.setParentName(getFillName);
        }
        if (StringUtils.equals(respBean.getPosterId(), entityId)) {
            respBean.setPosterName(getFillName);
        }
    }
}

Of course, visitor’s private method getFillName would be better implemented in AccountEntity. This way, your code will be refactored as follows:

List<AccountEntity> fillNameAccounts = agentDao.getAgentListByIds(accountIdList);
Visitor visitor = //get instance from some factory or singleton or app context ...

for (T bean : results)
{
    for (AccountEntity entity : fillNameAccounts)
    {
        bean.visit(visitor, entity);
    }
}

Have the two beans implement an interface and pull the methods in there.

Your beans might like a method

  setNameForEntity(String entityId, String nameToSet) 

as well.

This should compact your code a lot.

I think in this case you can extract some methods. Maybe this code

            if (StringUtils.equals(((AgentPostResponseBean) bean).getParentId(), entityId))
            {
                ((AgentPostResponseBean) bean).setParentName(fillName);
            }
            if (StringUtils.equals(((AgentPostResponseBean) bean).getPosterId(), entityId))
            {
                ((AgentPostResponseBean) bean).setPosterName(fillName);
            }

and this code

            if (StringUtils.equals(((ImportAgentPostResponseBean) bean).getParentId(), entityId))
            {
                ((ImportAgentPostResponseBean) bean).setParentName(fillName);
            }
            if (StringUtils.equals(((ImportAgentPostResponseBean) bean).getPosterId(), entityId))
            {
                ((ImportAgentPostResponseBean) bean).setPosterName(fillName);
            }

can be 2 private methods. Just use the “Extract Method” feature in your IDE. If you use IntelliJ IDEA or Android Studio, it’s Select the code -> Right Click -> Refactor -> Extract -> Method. If you use Eclipse, it’s Select the code -> Right Click -> Extract Method (I think. I haven’t used it in a while).