Modifying setter argument - is it hack or not?

Question

Is it normal to modify setter arguments? Let's imagine that we have setString method. And we really want to keep a trimmed form of the string. So a string with trailing spaces is invalid, but we don't want to throw an exception.

What's the best solution? To trim the value in the setter e.g.

public void setString(String string) {
    this.string = string.trim();
}

Or trim it in the caller (more than once) e.g.

object.setString(string.trim());

Or maybe something else?

Answer 1

Yes. After all, setters are designed for these kind of things! To control and sanitize the values written to fields ;)

Answer 2

Totally. Here's an example: suppose you have an engineering programs with different types of measurement units. You keep the internal values in one measurement system, but you convert from all others in the setter, and convert back in the getter, e.g.:

public double UserTemperature
{
  get
  {
    return Units.Instance.ConvertFromSystem(UnitType.Temperature, temperature);
  }
  set  
  {
    double v = Units.Instance.ConvertToSystem(UnitType.Temperature, value);
    if (temperature != v)
    {
      temperature = v;
      Changed("SystemTemperature");
      Changed("UserTemperature");
    }
  }
}

Answer 3

Yes, sure. Just be careful to check for NULL before applying any method (such as trim()).

Answer 4

There are two schools: one says its ok to check param in setter(school style) and second one says beans should not contain any logic and just data(enterprise style).

I believe more in the second one. How often do you look at implementation of your beans? should getUser throw any exception or just return null?

When you put logic in your setter and getters you make it harder to understand whats going on since many people will never look at its implementation. If you disagree I urge you to look at every setter and getter implementation before you use it just to check if its not just a bean.

Answer 5

At first glance it seems like it violates the principle of least astonishment. If I'm a user of your class, I'd expect a setter to do exactly what I tell it to. I'd throw an exception in the setter to force users to trim the input.

The other (better?) alternative is to change the name of the method to trimAndSetString. That way, it's not surprising behavior to trim the input.

Answer 6

Correct me if I'm wrong, but it looks logical to me that the setter should hold this kind of logic. If the setter is just assigning some value to an internal var without checking it, then why not expose the var itself?

Answer 7

This is exactly why you use setters rather than exposing the objects fields to the whole wide world.

Consider a class that holds an integer angle that's expected to be between 0 and 359 inclusive.

If you expose the field, calling functions can set it to whatever they want and this would break the contract specified by your API. It's also likely to break your functionality somewhere down the track because your code is written to assume a certain range for that variable.

With a setter, there's a number of things you can do. One is to raise an exception to indicate an invalid value was passed but that would be incorrect in my view (for this case). It's likely to be more useful if you modify the input value to something between 0 and 359 such as with:

actualVal = passedValue % 360;

As long as this is specified in your interface (API), it's perfectly valid. In fact, even if you don't specify it, you're still free to do whatever you want since the caller has violated the contract (by passing a value outside of range). I tend to follow the rule of "sanitize your input as soon as possible".

In your specific case, as long as you specify that the string is stored in trimmed format, there's no reason for callers to complain (you've already stated that such a string is invalid). It's better in terms of code size (not speed) to do it in the setter rather than at every piece of code that calls the setter. It also guarantees that the string is stored as you expect it to be - there's no guarantee a caller won't accidentally (or purposefully) store a non-trimmed string.

Answer 8

Yes. It is a feature of object oriented design is that the caller can treat your class as a black box. What you do inside is your own business, as long as the behavior of the interface is documented and logical.

Answer 9

While different people have different philosophies, I would suggest that property setters are only appropriate in cases where they will set an aspect of the object's state to match the indicated value and perhaps possibly notify anyone that cares about the change, but will not otherwise affect the object's state (it is entirely proper for a property setter to change the value of a read-only property if that property is defined in terms of the state associated with the property setter; for example, a control's read-only Right property may be defined in terms of its Bounds). Property setters should throw an exception if they cannot perform the indicated operation.

If one wishes allow a client to modify an object's state in some fashion not meeting the above description, one should use a method rather than a property. If one calls Foo.SetAngle(500) it would be reasonably expected that the method will use the indicated parameter in setting the angle, but the Angle property might not return the angle in the same form as it was set (e.g. it might return 140). On the other hand, if Angle is a read-write property, one would expect that writing a value of 500 would either be forbidden or else would cause the value to read back 500. If one wanted to have the object store an angle in the range 0 to 359, the object could also have a read-only property called BaseAngle which will always return an angle in that form.