-
Notifications
You must be signed in to change notification settings - Fork 232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add parameter to bypass setters #398
Comments
I just stumbled here. Agreed on the backward compatibility issue that using setters should be the optional behavior instead of a default behavior. |
Thank you for opening this issue. I did not expect that to be a breaking change, apologies for that. I can add the requested option and ship a new version asap. However, I do not agree with the following:
As a class designer, if I want to enforce some rules while setting fields to make sure my object's internal state is valid, I do not want Easy Random or any other library to bypass my setter by default and create invalid instances of my class, unless I explicitly tell the library to do so (See example in #400).
So IMO the option should rather tell the library to ignore/bypass setters instead of using them if they exist. Do you agree? As we all know, naming is hard.. I suggest |
I completely agree with your philosophy. It's actually why I started looking for an existing random instance library. My point was focused solely on backward-compatibility because easy-random is already bypassing the setters by using the reflected Field objects. I think this is the concern the original poster raises.
|
@PYangDizzle thank you for your feedback.
Indeed, that's a bug, see #400. Easy Random ignores the setter invocation failure and proceeds with reflection anyway (which should not be the case). But that's fixed now. I added a new parameter called
Of course, this will be documented in the wiki. @enedzvetsky wdyt? Looking forward for your feedback since you requested this feature. Nothing is final in the current snapshot, we can always rename the parameter or change the behaviour if you have a better suggestion. |
@benas
These points are difficult to address because they will deviate easy-random from the current behavior too much. I just want to ask the questions. |
Yes, if there is no setter, the field should be randomized by default, unless it is excluded by the user. You can use custom randomizers to override the behaviour.
Yes, I mean everything that might yield a
Easy Random was initially designed to randomize Java Beans only which by definition provide a setter for each property. This design decision turned out to be restrictive and many people started quickly asking to relax things and make it possible to randomize types that do not provide setters. If we remove the fallback to reflection when no setter is present, that would be a breaking change for all these people.. in addition to making the scope of the library very limited. Hopefully I answered all your questions. |
@benas Looks good. Setters should be used by default since we work with POJO objects. |
Perfect! we agree on the default behaviour and on the parameter name. Thank you for your feedback. |
Enhancement #388 breaks compatibility with previous versions.
Unfortunately, sometimes people add to setters business logic and this business logic doesn't support random data from easy-random.
Please add an additional setting to EasyRandomParameters like 'useSettersIfExist'.
The text was updated successfully, but these errors were encountered: