Should setters only set values?

Question

I always like to overthink things, so here's my thought of the day:

Should setters only set the values of class properties, and not perform any other logic?

Suppose I have a class

class User {
    private:
       int age;
       bool is_alive;
     public:
       int GetAge();
       void SetAge();
       bool GetIsAlive();
       void SetIsAlive();
};

When calling User::SetAge(5), one would assume that user.age == 5. But what about User::GetIsAlive()? Should setting the age of the user also perform internal logic to set the is_alive property, or should that be up to the user of the class?

Should it only do exactly what the function says?

void User::SetAge(int age){
    this->age = age;
}

or add additional operations?

void User::SetAge(int age){
    this->age = age;
    this->is_alive = (age < 100);
}

Should it be more explicit?

void User::DetermineIsAlive(){
    this->is_alive = (age < 100);
}

Obviously this is a simple case, but in my real world examples, doing something like setting the enabled state of a class (Device.SetState(false)) may then shut down sub components, update the status, etc..

I'm just curious if we think setters should ONLY set values or properties, and multi-statement functions should be more explicit in their naming.

Answer 1

The overall problem you're having is that you're letting the implementation of this object guide its interface. That's almost always a bad idea (and is pretty much only justifiable for performance reasons).

This almost certainly stems from a lack of understanding of what this object is for. What purpose it serves within the overall design of your system.

Take your example of considering setting the internal is_alive field based on the age. You're asking if it is right or not to do that in SetAge. Well... you're asking the wrong people and the wrong question. The right question is "what is the relationship between age and being alive, and who decides what that is?"

Does the essential nature of User include the idea that it must not be alive if its age happens to be 100 or more? If so, does this also mean that any User whose age is less than 100 must also be alive? The only person who can answer these questions is yourself, and every question's answer will in some way dictate the interface of the class.

But the overall point is this: none of these questions has anything to do with how you store the values. These questions are not about "accessors" or whatever; they are about creating a type with a meaningful interface and a consistent, correct design towards some purpose.

This is not a problem of "overthinking"; it's a problem of wrongthinking, focusing on the wrong thing. Figure out what the type is supposed to do, then build a type that does that.

Answer 2

Should setters only set the values of class properties, and not perform any other logic?

No.

Following this rule makes having setters pointless. The only reason we put up with this tragic preemptive clutter is because some languages (also looking at you Java) have forced us to provide setters even when their side effects don't exist yet because allowing direct assignment to public fields establishes an interface that would have to change to allow us to add side effects like validation. Other languages, like C#, allow the using code to stay the same: o.x = -1; In C# that might throw an exception at you. In C++ that would have to be o->setX(-1); to do that.

A client shouldn't have to change to add this. That's why, when you're stuck in a language that doesn't let you add these later, you have to start out with setters. Even when you don't need them now. This is manual aspect oriented programming for languages that can't do it themselves. We do this so we get the chance to do something before or after the value is set. Even if we don't know what that something is going to be yet. Even when we don't need it now. We're doing this to be prepared for when we do.

We create setters that have no side effects only because our language doesn't have the flexibility to let us cleanly add them later without breaking clients. If you forbid ever adding the side effect this whole annoying exercise is pointless.

Well, except for breakpoints. They can be handy places to set breakpoints. But other than that, this makes them pointless.

Answer 3

Side effects are OK and one of the reasons for having properties instead of straight public member variables in the first place. Your example is not compelling though. But you can think of a private update method that gets called each time a property value is set. The update method could change some color in the UI or perform a new search using the new property value.

But... it is a bit of a convention not to do heavy lifting in either a property getter or setter because that is not expected. Getters and setters should be quick and safe. If there is a lot of work involved with the possibility of throwing exceptions, you are supposed to put it in a regular method.

Answer 4

Given that being alive is not guaranteed when being 90 years of age. I propose the following class as an example where I believe that having side effects is usefull.

class User {
   private:
      int age;
      bool is_minor;
   public:
      int GetAge();
      void SetAge();
      bool GetIsMinor();
     // void SetIsMinor();  // this function is not needed and should
                            // be removed
};

void User::SetAge(int age){
    this->age = age;
    this->is_minor = (age < 16);
}

I hope this example can help explain how it can indeed be usefull to have some side effects inside a set function. But as explained by others, no heavy lifting inside a 'set'.

Answer 5

For me, this is about communication. If I took your question by word:

Should setters only set values?

I would say, they should because one expects a setter to be something that sets a value.

I would distinct these two:

1. what is thought to be a setter - intention in creation
2. what looks like a setter - the code has a commonly used naming pattern

(1) I will recognize that I want to create a setter, if I like to access members of an object without asking it for much permission. As indicated by Onorio Catenacci, this might violate data hiding and the Law of Demeter. Consider, I created a raw setter and now I add logic to it. Maybe this logic has more meaning to me: setAge() could become getOlderOneYear() or celebrateBirthday() where the second one would commonly have the side-effect of inviting other objects to the Party. Then, this is not a setter by intention any more. It might be, I use the same form because I do not refactor my code but the meaning changed from just setting values.

(2) a setter by form is something you can see in the code and you interpret it to get meaning from it. For me, it is a question of what you expect. If you read the object not as an object for conversation but just as a data store, you expect minimal side effects. As an example, I look at clock.setTime(time). If I expect it to be a setter, I think, it stores the value of time inside the clock object. However, I would like my clock to call this->minutePointer.moveTo(...) and this->hourPointer.moveTo(...). I think, I better use clock.display(time) if you think setTime does not have side effects.

To summarize this: There is a difference between a setter by intention and a setter by form. If intention and form align, you have a clear communication. If they differ, it is one step away from a common vocabulary. The computer does not mind your intention. Other people may care.

I would call it setAge or ageByYears(...) depending on the what I want to communicate. Can I really get younger?

When do I use setters? I rarely use setters. When I do, I think, I remember a time, when I used them privately inside an object to validate that the object coming in fulfills certain contracts with assertions when the setting of the value comes from two different places.

Answer 6

Beyond simply assigning the value to a member variable, a setter should validate the value beyond the simple type checking.

For example, if I have a percentage value stored as an integer, then chances are I want to check the value passed in to insure it's no more than 100 and no less than 0. Even if I use an unsigned int I still need to check to insure it's not more than 100.

All that said, setters and getters strike me as an anti-pattern. While they seem to help, they actually lessen data hiding. The fact that having a getter and/or setter exposes the type of the private member weakens data hiding. So avoid getters and setters unless you absolutely must have them. You might as well make the member variable public.