Skip to content
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

Randomize existing bean #149

Closed
lbeuster opened this issue Jun 14, 2016 · 16 comments
Closed

Randomize existing bean #149

lbeuster opened this issue Jun 14, 2016 · 16 comments
Assignees
Labels

Comments

@lbeuster
Copy link

lbeuster commented Jun 14, 2016

Hi,
it would be nice if it would be possible to randomize an existing bean. Sometimes it's necessary to do the bean creation by yourself.

Either provide explicit methods to populate one existing bean or make the randomizer more extensible. Perhaps make more config objects public and move their creation (e.g. the ObjectFactory) to the builder and pass them to the randomizer.

What do you think?

Best regards
Lars

@fmbenhassine
Copy link
Member

Hi,

Thank you for this feature request. I see the idea.

Either provide explicit methods to populate one existing bean or make the randomizer more extensible.

I would go for providing a public API to randomize an existing bean. Let me come up with a working prototype and keep you informed. You are welcome to contribute if you want.

Kind regards
Mahmoud

@fmbenhassine fmbenhassine added this to the 3.2.0 milestone Jun 15, 2016
@fmbenhassine fmbenhassine self-assigned this Jun 15, 2016
@fmbenhassine fmbenhassine modified the milestones: 3.3.0, 3.2.0 Jul 5, 2016
@fmbenhassine
Copy link
Member

I was working on adding a method to randomize an existing bean, but I struggled to find a good method name.. This is because EnhancedRandom is designed as an extension of java.util.Random and all methods expect a type and not an already created object.

So I'm a bit confused if this feature should really go into the library or left to the developer (may be create a random instance of the object's type and copy the desired fields?) ..

@JesusMetapack
Copy link

JesusMetapack commented Aug 19, 2016

Why not have the option to add to the EnhacedRandomBuilder a Map of Supplier instances, each one associated to the type of object to be created by the EnhancedRandomImpl. That way the user can inject default values to the created instances, so stops the randomizer from producing values for those.
Sometimes default values are not defined in third party classes, so this would allow to use the iOverrideDefaultInitialization option for types with not provided default values
This would allow to customize part of the bean with predefined data, and the rest of the bean would be random

@fmbenhassine fmbenhassine removed this from the 3.3.0 milestone Sep 23, 2016
@fmbenhassine
Copy link
Member

Why not have the option to add to the EnhacedRandomBuilder a Map of Supplier instances, each one associated to the type of object to be created by the EnhancedRandomImpl.

This is possible by registering a custom registry: https://github.com/benas/random-beans/wiki/Grouping-Randomizers

@sansherbina
Copy link
Contributor

I think, that library must have ability to randomize existed object. We already have private method randomize in class EnhancedRandomImpl. I suggest to create public method randomize which has parameters Object and List exclusion fields.
Also, I agree, that library should provide more public api. Because, when I met with this library first time, I would like to customize it to my project. And all classes are package-private, so it is impossible to customize it. I like use Spring because there, developer can customize everything. I think it is better to use factories instead of direct constructing object by using operator new.

@petrholik
Copy link

Hello I've recently facing issue: I've got DTO which assigns default values like this:

public CurrencyDTO(Long id) {
		this.id = id;
		this.finRefund = new BigDecimal(0);
		this.finSum = new BigDecimal(0);
}

It is not possible to properly randomize this object because there is:

if (getFieldValue(result, field) != null)
in populateField method.

It would be nice and useful feature to be able randomize this sort of object. I am writing it here because i think is related.

@PascalSchumacher
Copy link
Collaborator

@petrholik I guess for this usecase it would suffice to add an option for overwriting existing field values during randomization.

@petrholik
Copy link

Definitely, should I make PR implementing it?

@PascalSchumacher
Copy link
Collaborator

@petrholik @benas is the boss, so he gets to decide, but I think a PR would be welcome.

@fmbenhassine
Copy link
Member

Hi @petrholik,

Sure, you are welcome!

Please open a separate issue with the failing example.

Kind regards
Mahmoud

@fmbenhassine
Copy link
Member

fmbenhassine commented Dec 2, 2016

Hi @sansherbina

I think, that library must have ability to randomize existed object.

Randomizing an existing bean was out of scope of RB in first place.

As I said in my previous comment, RB has been designed as an extension of java.util.Random class. All methods in java.util.Random expect you to specify a type and get a random instance (int, double, etc). To be consistent with it's base class, EnhancedRandom expects a type too, not an instance.

Even though it is (of course) possible to make RB randomize existing objects, for API consistency, I'm not sure this feature should be added to the library.

Also, I agree, that library should provide more public api. Because, when I met with this library first time, I would like to customize it to my project. And all classes are package-private, so it is impossible to customize it.

As of v3.4, RB provides:

  • 12 parameters to configure it's behaviour
  • Randomizer/Supplier APIs as extension points for custom randomization logic
  • RandomizerRegistry API as extension points for custom field/type to randomizer mapping
  • Custom exclusion points with FieldDefinition API or custom annotations

How would you like to customize it to your project without being able to use one of these extension points? Can I help?

Kind regards
Mahmoud

@sansherbina
Copy link
Contributor

For my project I needed option for control recursion. I searched possibility to customize library without contributing changes to library. But this was impossible. In new version we already have this. So, my problem is resolved.
Spring framework provides possibility to customize everything, by overriding beans. So, sometimes this customizations make project more complicated, but sometimes it is really useful.
I think for library better have this.
I am talking about class EnhancedRandomImpl. Now in constructor this class fully initialized. I think better provide any factory or something else, to allow substitute each filed
private final FieldPopulator fieldPopulator;
private final ArrayPopulator arrayPopulator;
private final CollectionPopulator collectionPopulator;
private final MapPopulator mapPopulator;
private final Map<Class, EnumRandomizer> enumRandomizersByType;
private final RandomizerProvider randomizerProvider;
private final ObjectFactory objectFactory;
private final FieldExclusionChecker fieldExclusionChecker;
And make this classes public for clients, to allow extends from them.

@fmbenhassine
Copy link
Member

fmbenhassine commented Dec 3, 2016

For my project I needed option for control recursion. I searched possibility to customize library without contributing changes to library. But this was impossible. In new version we already have this. So, my problem is resolved.

Random beans is not perfect. There are many rooms for improvements. You asked for a feature, we've added and we are happy we solved your problem 😄 We've implemented this feature by adding an extension point, not by allowing you to modify an internal class. We try to follow the open/closed principle. So this:

all classes are package-private, so it is impossible to customize it

is not completely true. Spring is so flexible because it provides a lot of extension points. But it has hundred of internal classes that you cannot override.

I am talking about class EnhancedRandomImpl. Now in constructor this class fully initialized. I think better provide any factory or something else, to allow substitute each filed

Indeed! This class has many collaborators and probably should be split. But it acts as a mediator by delegating and coordinating work among all collaborators. We've tried to make every collaborator do one thing, and at the end, we need some class to coordinate the core work, which is EnhancedRandomImpl.

Anyway, I agree with you and I've got the idea of making some internal collaborators public and "overridable". We need to define a clear contract for every component, provide a default implementation, and let the user override it if needed. I'll try to proceed this way for the next major release.

@fmbenhassine
Copy link
Member

fmbenhassine commented Dec 5, 2016

Hi,

After re-reading this thread, I came to the conclusion that we actually have two approaches: making things configurable vs making things overridable (or replaceable).

Currently, RB is probably trying to make everything configurable by adding configuration/extension points but not allowing to override or replace internal components.

The other approach would be to provide default behavior and make components completely overridable/replaceable by the user.

@sansherbina You are right, the second option is likely to be more flexible. It's even easier from the point of view of the library maintainer! Instead of trying to imagine every configuration option that every user requests.. try to define a clear contract for every component, provide defaults and make them overridable (That is, program to interfaces). This requires more design effort at first place but it leads to more flexible library and cheaper maintaince in the long term. I'll try to improve the design of RB in this direction for next major release.

Best regards
Mahmoud

@sansherbina
Copy link
Contributor

Hi Benas,
You are right. If you want, I can help you with this redesign. This task looks interesting for me. For example you can create document where briefly describe architecture and plan. And then, developers can help you with this work.

@fmbenhassine
Copy link
Member

This is because EnhancedRandom is designed as an extension of java.util.Random and all methods expect a type and not an already created object.

I'm closing this issue for now. I prefer keeping the API consistent with the base class which starts from a type and returns a random instance. The suggested feature can be implemented in a fork. OSS FTW!

Nevertheless, I retain all the good design ideas discussed here and will try to implement them in the next major release v4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants