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

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

Answer 3

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 4

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 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.