Is It Bad Practice to Make a Setter Return "This"

Is it bad practice to make a setter return this?

I don't think there's anything specifically wrong with it, it's just a matter of style. It's useful when:

  • You need to set many fields at once (including at construction)
  • you know which fields you need to set at the time you're writing the code, and
  • there are many different combinations for which fields you want to set.

Alternatives to this method might be:

  1. One mega constructor (downside: you might pass lots of nulls or default values, and it gets hard to know which value corresponds to what)
  2. Several overloaded constructors (downside: gets unwieldy once you have more than a few)
  3. Factory/static methods (downside: same as overloaded constructors - gets unwieldy once there is more than a few)

If you're only going to set a few properties at a time I'd say it's not worth returning 'this'. It certainly falls down if you later decide to return something else, like a status/success indicator/message.

Why is it bad practice to allow to set a collection?

The logic behind set and get operations is to allow validation or replacing of inner representation, if you let an external class set the specific implementation, you lose control over the insertion logic (allows duplicates? is ordered?, is mutable?), and you make you object harder to use, as the users of it have to decide that, when is very probable that they don't care.

Is it bad practice to have my getter method change the stored value?

I think it is actually quite a bad practice if your getter methods change the internal state of the object.

To achieve the same I would suggest just returning the "N/A".

  • Generally speaking this internal field might be used in other places (internally) for which you don't need to use the getter method. So in the end, the call to foo.getMyValue() could actually change the behaviour of foo.

Alternatively, the translation from null to "N/A" could be done in the setter, i.e. the internal value could be set to "N/A" if null is passed.


A general remark:

I would only add states such as "N/A" if they are expected by some API or other instance relying on your code. If that is not the case you should rely on the standard null types that are available to you in your programming language.

Is object creation in getters bad practice?

Properties look like fields but they are methods. This has been known to cause a phenomenal amount of confusion. When a programmer sees code that appears to be accessing a field, there are many assumptions that the programmer makes that may not be true for a property.So there are some common properties design guidelines.

  1. Avoid returning different values from the property getter. If called multiple times in a row, a property method may return a different value each time; a field returns the same value each time.

  2. A property method may require additional memory or return a reference to something that is not actually part of the object's state, so modifying the returned object has no effect on the original object; querying a field always returns a reference to an object that is guaranteed to be part of the original object's state. Working with a property that returns a copy can be very confusing to developers, and this characteristic is frequently not documented.

  3. Consider that a property cannot be passed as an out or ref parameter to a method; a field can.

  4. Avoid long running property getters. A property method can take a long time to execute; field access always completes immediately.

  5. Avoid throwing exceptions from getters.

  6. Do preserve previous values if a property setter throws an exception

  7. Avoid observable side effects.

  8. Allow properties to be set in any order even if this results in a temporary invalid state of objects.

Sources

"CLR via C#", Jeffrey Richter. Chapter 9. Defining Properties Intelligently

"Framework Design Guidelines" 2nd edition, Brad Abrams, Krzysztof Cwalina, Chapter 5.2 Property Design

Using setter methods in constructor: bad practice?

It's probably not a good idea. If you don't make that class final and don't make the setName( ... ) method private or final someone else is able to extend your class and overrid the setName( ... ) method. Your constructor (in your base class) will call that method in the extending class instead of your implementation. Nobody knows what that method can do. As a rule of thumb: a constructor shouldn't call methods that can be overriden.

Setter conventions in Java (return void or this)

Method chaining may look nice in some situations, but I would not overuse it. It does get used a lot in the builder pattern, as mentioned in another comment. To some degree it's probably a matter of personal preference.

One disadvantage of method chaining in my opinion is with debugging and breakpoints. It may be tricky to step through code filled with chained methods - but this may also depend on the IDE. I find the ability to debug absolutely crucial, so I generally avoid patterns and snippets that could make my life harder when debugging.

Is Java breaking its own rules with setter methods that have non-void return types?

A setter is usually used as an accessor method to change the state of an object. As Kayaman mentioned it can be useful to return the object itself to create a fluent api instead of void - see it as syntactical sugar, when working with these objects.

Nevertheless, in the NIO case, it is a static method of the class "Files" that changes the state of the Path that is given as first argument - so in this case it's no accessor but a "normal" method that has "set" in its name.



Related Topics



Leave a reply



Submit