Should method names getX and setX only be used for fields and have no other effects?

Question

Should I avoid using getX() and setX() as names for methods that aren't "traditional" getters or setters? (Let's define traditional as it only gets/sets the field and has no other effects.) I guess the question is does a method being named as such imply that it's a field of the object? Also, is it bad practice to do other things in them?

class Foo {
    private Object bar;
    private Object bazz;

    // "traditional"
    public Object getBar() {
        return bar;
    }

    // bad practice?
    public Object getBazz() {
        doSomeOtherThing();
        return bazz;
    }

    // not a field
    public Object getJazz() {
        return makeSomeJazz();
    }

    // ...
}

If getX() isn't appropriate if it's not a field, what is? (Probably too broad, I realize.) Obviously something like makeJazz() is probably better than getJazz() if getJazz() is considered bad here.

I guess another question is does it really matter? I like the idea of "self documenting code", but maybe I'm just worrying too much about something minor.

@rwong haha, true about that, but I did have other points in the question. — Captain Man, Apr 30 '15 at 20:34

score 5 · Accepted Answer · answered Apr 30 '15 at 21:13

Let's say you're writing a GUI control, and you want to be able to resize it. Intuitively, you should have a setHeight method and a setWidth method for the dimensions. These should set a value on the control, but if they don't also cause the control to recalculate and redraw itself, you're violating the heck out of POLS, because your user is going to expect changing the size of the control to visibly change the size of the control. (Not to mention leaving the control in an inconsistent state, because the backing field value doesn't match what's shown on screen.)

Instead of "set a value and have no other effects," your rule of thumb should be "do what is necessary to bring the object into a fully consistent state, but nothing more." Sometimes this will only require setting a value. Other times, it'll take a lot more work.

score 2 · Answer 2 · answered May 01 '15 at 07:24

Requiring that getX methods always correspond to a field breaks encapsulation.

That the field exists at all is private information of the class, something no other class should rely on. Maybe you want to change to a different implementation some day. On the other hand, the getX method is a public interface. If it implied the existence of a field, would it have to be removed if the field was?

Let's say a person's full name is always of the form "Firstname Lastname" (obviously they aren't, but this is an example). You could store that with two fields, firstname and lastname, or with one field name. Both those implementations allow functions get_first_name, get_last_name and get_full_name to exist. That's fine.

A getX method should behave as if it were just a field lookup: it should be fast, it should be constant (if the object didn't change in the meantime), it should have no unnecessary side effects, and if a setX method exists then it should accept whatever value was returned by getX and behave like a setX method.

Should method names getX and setX only be used for fields and have no other effects?

2 Answers2