How to (or should you) avoid long methods/classes

Question

I'm mostly working with legacy code in a JSF based project and there are lots of quite long classes and methods in backing beans.

This is constantly bugging me but when I look for what can be done, most of the time all I can come up is to divide a long method to n small methods. Which gives you still a very long class and sometimes harder to read too.

So what do you do to keep your backing beans short and concise? Or do you keep one big backing bean for one page? Are there any best-practices?

I assume this is not directly related to jsf but to any model where you are 'backing' up your view with a controller. So a general advise would be helpful also.

Answer 1

Not only applicable to backing beans, but to all objects in general:

you should always refactor long methods by extracting them into multiple smaller ones.

Answer 2

IMHO, each method should be 1 step, if that 1 step conatins multiple inner steps you should break them down into smaller methods so its easier to read. The good thing with modern IDE's is that they refractor code for you without much effort at all.

Answer 3

Put all fields in its own class, also called an entity or DTO class (e.g. User, Product, Order, etc) and reference it instead. Those can be JDBC/JPA entities. Put all business methods in its own class, also called a service or domain object (e.g. UserService, ProductService, etc) and reference it instead. Those can be EJB's.

E.g. thus not

public class Bean {

    private Long id;
    private String field1;
    private String field2;
    private String field3;
    // Etc... All model fields.

    @PostConstruct
    public void init() {
        // >20 lines of business/domain code to prefill the fields from DB.
    }

    public void save() {
        // >20 lines of business/domain code to save the fields in DB.
    }

    // Getters/setters for all fields and possibly also all nested fields.
}

But more so

public class Bean {

    private Long id;
    private Entity entity;

    @EJB
    private EntityService service;

    @PostConstruct
    public void init() {
        entity = service.find(id);
    }

    public void save() {
        service.save(entity);
    }

    // Getter/setter for id and getter for entity.
}

Further, I've also seen code wherein the nested objects/entities are delegated through by additional getters/setters in the bean like so

private Entity entity;

public String getField1() {
    return entity.getField1();
}

public void setField1(String field1) {
    entity.setField1(field1);
}

// Etc...

This is totally unnecessary. Just a single getter for the entity is sufficient (setter is not mandatory!), in combination with

<h:inputText value="#{bean.entity.field1}" />

The entities at its own can also be further divided. E.g. street, houseNumber, zipCode, city, country of an User could be replaced by another entity/DTO Address within the same User.

---

If you have bad luck, the code has been autogenerated by a visual editor (e.g. Netbeans + Woodstock). There's not much to refactor anyway without completely redesigning it, I would rather look for another project.