10

other dev changed constructor from

public A(String val){
this.val = val;
}

to

public A(String val){
this.setVal(val);
}

this feels bad in many ways, but need some arguments to convince him that its wrong

Comments
  • 8
    I sometimes follow this approach if I set some validation inside the setVal function
  • 4
    @gitpush

    no validation in the setter here

    just plain

    public final void setVal(final String val) {

    this.val= val;

    }
  • 1
    @tkdmatze in this case you can start with:

    1. The need for getter and setter so that out side world directly modify the var, but instead setters are used so that the world give their value and if the class needs certain requirements then they are validated inside the setter

    2. Constructor is assigning values for the first time and not updating an existing value, so that directly modifying that var is safe without using setter (assuming someday you add validation so that he doesn't ask what if we add validation later on)

    That's what I could think of, hope they are enough
  • 1
    Isn't the setter role to set a variable and the whole idea is that you change in only one place?

    So its actually good. If you vhange the variable name you just need to change in the setter not in constructor as well.
  • 1
    What are your arguments for it feeling bad in many ways?
  • 2
    @Kifflom @curlyDev for me I find it bad using it in a constructor if there is no validation inside the setter function, it is not that it negatively effect the code but assume there are read only vars that are only set during init of the class, imagine having:

    this.setX()

    this.y =

    it is a mess to read IMO
  • 2
    @Kifflom

    to give it a little more context, its a DTO, an immutable object, properties have XML-Validation Annotations

    the only way it is initalised, is through the constructor, setters are unnecessary because never used until the change now ... but other dev wanted them ...

    Validation is done object wise not propertywise after the object is created via constructor
  • 4
    Calling instance functions from the constructor is officially wrong, as not all the variables are initialized
  • 3
    1. If the class is not final no public method should call another public method. By public I mean non-private and constructor also counts as a method. The reason is pretty simple. Imagine someone extends your class and changes behavior of that setter. The behavior of constructor will change too, but the person who overrode the setter will not expect that and will get weird unexpected bugs. Or if he wants the constructor to stay the same and override the setter he might not be able to do that at all.
    2. Calling function will be a little bit slower than just assigning the value directly.
  • 1
    it it is c# and using properties with properties changed notification, you do not want to have notifications before the object is constructed.
  • 1
    @Getter @Setter @Data annotations...
    People still write setters by hand?

    Also, validating data inside a setter feels wrong on so many levels to me @gitpush. If I use a getBlah or setBlah method I expect it to do exactly that and not more or less.
Add Comment