-
No, we should not allow private setters in this case. Those methods are called from a different class and may be called on the unknown thread so their implementation should be prepared for that - the same as it was a public member of a class. By allowing private setters, coders implementing setter method could expect that it is called only from the class and would omit proper protections in the implementation.
-
I'm not sure I agree with that logic - following it we should mandate that all members annoted with
@Inject
should be public… In case of Kernel setters may be used to affect injection process (doing some additional work), and while they may be called "from the outside" (i.e. Kernel) for the sake of setting everything up, we don't necessarily may want to exposed setting some member directly.This was an afterthought of fix for #helpdeskpr-174 (commit:
c220607bfa5fb78bd2c25e12f1ac496673417f7e
). -
To be honest, any extensive work in setters for
@Inject
fields should not be done. This is a possibility to be able to do some work like setting a value of other field or calling initializer after all fields were injected.I do not think that exposing setters is bad in this case, as with use of kernel, configuration and injection the same thing could be done with
private
setters, so having them marked as public looks ok. -
To be honest, any extensive work in setters for @Inject fields should not be done. This is a possibility to be able to do some work like setting a value of other field or calling initializer after all fields were injected.
I completely agree on both points, wasn't arguing against that.
I do not think that exposing setters is bad in this case, as with use of kernel, configuration and injection the same thing could be done with private setters, so having them marked as public looks ok.
I don't see how that can be achieved with only configuration and injection. Yes, you can configure different class for injected member but that would (roughly) stay the same during the whole lifecycle, and if you inject a class with private member/setter, without reflection I don't see changing that private member. If we force public setter (in case when we want/have to do some light work on private member) then we expose it to programmatic change.
It boils down to two things basically, in case we want to stick with public setter:
- add it as a note to documentation;
- (maybe) inform developer, that it has a setter for injected member, but it's private so it won't be called (warning? exception?)
Figuring it out with debugger was kinda PITA…
-
We could add information to the documentation and possibly warn developer at runtime with a warning. @bmalkow What do you think about that?
Type |
Question
|
Priority |
Normal
|
Assignee | |
Subsystem |
Kernel
|
Not sure if this is a but per se, but I just stumbled against rather odd issue - it turned out that setters associated with private fields annoted wit
@Inject
requirepublic
access modifier. Is it intentional? Shouldn't we allow private setter methods in this case?