-
-
Notifications
You must be signed in to change notification settings - Fork 509
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
UseConstructor(), SkipConstructor() and fixing StrictMode() error #338
base: master
Are you sure you want to change the base?
Conversation
|
||
|
||
[Fact] | ||
public void UseContructor_After_SkipConstructor() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is debatable if using both UseConstructor
and SkipConstructor
should be prohibited or not although it does not break anything.
#else | ||
//Not sure wich works better | ||
//this.CreateActions[Default] = (Faker faker) => (T)FormatterServices.GetUninitializedObject(typeof(T)); | ||
UseConstructor((Faker faker) => (T)System.Runtime.Serialization.FormatterServices.GetUninitializedObject(typeof(T))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this.CreateActions[Default] should be called or this.CreateActions[currentRuleSet] instead.
{ | ||
if( populateActions.TryGetValue(propOrFieldOfT, out var populateAction) ) | ||
if (populateActions != null && populateActions.TryGetValue(propOrFieldOfT, out var populateAction)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioned bug was here when no RuleFor() was called (Or simple AddRule) when StrictMode(true) the populateActions variable was null and resolved in null refernce exception
@Basyras Thank you for the PR. I really appreciate it! I'll try to provide my thoughts below.
If you can, please break up the PR. There are too many changes in this PR. I will try to dedicate more time for this during the holiday break. Thanks, |
@bchavez Shared formatting rules from .editorconfig file were not applied. I can see .editorconfig file in your root folder but i think your formatting rules are not inside. That's the reason why my formatting changes were made. Could you fix that? Should be simple, go to your IDE formatting settings and on top there should be option to generate .editorconfig |
ae40cad
to
67a312d
Compare
What ever happened to this PR, I don't seem to see it ever made it into the code ? |
@gl1tch nope |
0e50b51
to
0be1f8e
Compare
0b65acc
to
cc799df
Compare
@bchavez Promised PR from #334
➕ Adding features for specifying constructors:
UseConstructor()
andSkipConstructor()
UseConstructor() also provide overload for specifying if paremeterless constructor is private/protected
U have already discussed if this should be in Bogus here: Using Faker[T] with private constructor and private property setter #213 - i think it makes sense to to have this feature because using SkipConstructor() is way more shady then asking users if they want use hidden constructor
Usage:
UseConstructor()
to use public parameterless ctorUseConstructor(false)
to use public parameterless ctorUseConstructor(true)
to use private/protected parameterless ctorUseConstructor(x=>new Instance())
to use custom delegate (CustomInstantiator()
marked as obsolete)SkipConstructor()
to not using constructor at allWhen parameterless ctor is not found the NoParameterlessCtorException is thrown
🔧 Found & Fixed bug when StrictMode(true) is used and no RuleFor() is specified
⚠️ .net standard 1.3 is missing few methods so
NotSupportedException
is thrown istead in this assemblySorry for using different code formatted in advanced ... 😃
Also question is it necessery to have T in Faker[T] to be reference type? Because i would consider generating structures handy and looking very briefly i didn't see any reason why it would not be possible