A setter with logic in java

Question

Consider a Message object in Java that stores some text.

public class Message {

    private String text;
    private boolean containsDigit;

    public Message() {
        //constructor
    }

    public String getText() {
        return text;
    }

    public void setText(String text) {
        this.text = text;
    }

    public boolean isContainsDigit() {
        return containsDigit;
    }

}

This is a persisted object.

I have no problem with data being set in the constructors but the Message's text field can be set after the object is created and when the text is set, the containsDigit field should also be query-able thereafter. The obvious way to do this is in the setter:

public void setText(String text) {
     // presume existence of method to check for digit
     if(text.containsDigit())
         this.containsDigit = true;

     this.text = text;
}

But does this result in any "best practice" alarms bells going off due to having logic within a setter method?

Would anyone suggest an alternative implementation?

EDIT

I should probably add that the containsDigit field is required because the object is persisted so the containsDigit field can be queried subsequently. Also, in an application using the Spring/Hibernate engine, this setter is constantly called when re-reading/writing the object so was wondering about the practicality/efficiency of this also.

Answer 1

Your case is the very reason for using setters and getters. If you weren't allowed to have logic in a setter, then you might as well access the fields directly!

Answer 2

It is probably not the best practice, however sometimes the life is stronger than best practices.

Probably better approach is to remove field containsDigit and move the logic you added to setter into isContainsDigit():

public class Message {
    private final static Pattern d = Pattern.compile("\\d");
    private String text;

    public String getText() {
        return text;
    }

    public void setText(String text) {
        this.text = text;
    }

    public boolean isContainsDigit() {
        return text == null ? false : d.matcher(text).find();
    }

}

Answer 3

Implement the method public boolean isContainsDigit() as following:

public boolean isContainsDigit() {
    return getText().containsDigit();
}

This way you do not have to keep them both in sync, while having to reevaluate it again and again. On the other side, never performance optimize your code, if you have no need to do it. The method setText() and isContainsDigit() fall under racing conditions, if they are accessed concurrently by two threads. Perhaps you have to synchronize them if you want to address this issue.

Answer 4

@dre - really its too bizarre. neither of the links that Saint linked to differentiate themselves substantially from your question other than to argue about some kind of design elegance or purity. If you have a "setter" then the whole point is to be able to surround the setting of the variable with some logic that ensures that the variable gets set correctly taking into account dependencies that exist to other attributes in the class provided such need arises. If "setters" are going to be "pure", then get rid of them and make the variable public. There might actually be something in Robert Martin's book "clean code" that explicitly speaks to the principle of the occam's razor or the principle of parsimony. Perhaps this link helps : http://effectivesoftwaredesign.com/2013/08/05/simplicity-in-software-design-kiss-yagni-and-occams-razor/. Finally it should be noted that it is not at all recommended to make an attribute public since going back and making an attribute private from public is a nightmare. i reiterate - first choose to make your class immutable with all member attributes private. If users of the class scream, ask them to justify why they need to mutate it. Once their justification is taken into account, create a setter, do not make the variable public. That should work as a set of design rules. Hibernate or JPA should have nothing to do with this design decision.

To make a class that has many parameters immutable use the builder pattern - When would you use the Builder Pattern?

Answer 5

I can say that this is "good enough". However, if you want to follow object oriented design completely you will implement something like event onTextChange, that will be fired whenever set changes text. On the other hand you will implement handler for this event that will update containsDigit field. For your simple example this is really unnecessary but if you expect adding new logic in future in setText() you can consider this.