j-easy / easy-random

The simple, stupid random Java beans/records generator
https://github.com/j-easy/easy-random/wiki
MIT License
1.61k stars 232 forks source link

throw java.lang.InstantiationException instead of returning null #39

Closed kermit-the-frog closed 8 years ago

kermit-the-frog commented 9 years ago

Hi,

jPopulator catches every java.lang.InstantiationException and returns null if the instantiation fails. I would prefer that my tests fail when the test data is incomplete. Therefore, I would be great, to be able to switch the behavior at InstantiationExceptions.

Best regards Kermit the Frog

fmbenhassine commented 8 years ago

Hi,

Sorry for the late reply. Could you please give an example of when the instantiation fails? I would like to see how jPopulator returns null.

What do you mean by "switch the behavior at InstantiationException"? Would it be throwing an exception or making this behavior configurable (to let the user choose between returning null or throwing an exception) ?

Regards Mahmoud

fmbenhassine commented 8 years ago

Hello,

I was thinking of making the behaviour configurable in this way:

Populator populator = PopulatorBuilder.aNewPopulator()
   .silentMode(true) // or may be another method name
   .build();

This would (silently) swallow exceptions and return null when the instantiation fails. If silent mode is set to false, the populator will throw a runtime exception instead of returning null.

The second option is to update the Populator API to throw an exception:

public interface Populator {
    <T> T populateBean(final Class<T> type, final String... excludedFields) throws BeanPopulationException;
}

The client code will be able to catch the exception and know that bean population failed.

Would love to hear your thoughts.

Kind regards Mahmoud

kermit-the-frog commented 8 years ago

Hello Mahmoud

sorry for not replying to your first message. I'm on holiday for two weeks.

Thanks for thinking about my suggestion. Both options are ok for me. You should choose the one you prefer.

Kind regards, kermit-the-frog

fmbenhassine commented 8 years ago

Ok that's fine.

I'll go for adding an exception clearly in the API.

Have a nice holiday :smile:

fmbenhassine commented 8 years ago

Hello @kermit-the-frog

I've added the BeanPopulationException in the Populator API. This exception will be thrown whenever a bean can not be populated. You can catch this exception and make your test fail as requested.

Kind regards Mahmoud

kermit-the-frog commented 8 years ago

Sorry for the late response. Thanks for the Exception. The implementation as a runtime exception is perfect. The only thing that could be better is the message of the exception. It's hard to discover which field in which class causes the exception. Especially if you have serveral fields of the the same type in your classes.

fmbenhassine commented 8 years ago

Hi,

Thanks for the Exception. The implementation as a runtime exception is perfect.

Great! glad to hear that.

The only thing that could be better is the message of the exception. It's hard to discover which field in which class causes the exception. Especially if you have serveral fields of the the same type in your classes.

Sure! Please open a separate issue with all these details and I'll plan it for next release. Thanks

Regards Mahmoud