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

Support for non-public setters/constructors. #179

Closed
mcosta91 opened this issue Sep 27, 2018 · 9 comments
Closed

Support for non-public setters/constructors. #179

mcosta91 opened this issue Sep 27, 2018 · 9 comments

Comments

@mcosta91
Copy link

Hi! Great work so far with Bogus!

Bogus currently doesn't work with non-public parameterless constructors/setters. Is this behavior by-design? If not, I would like to work on that issue.

Thanks in advance.

@bchavez
Copy link
Owner

bchavez commented Sep 27, 2018

Hi Marcus!

Thanks for the kind complement! I'm always happy to hear from developers using Bogus! 👍 I'm always interested in seeing how we can make Bogus better.

I would probably say the decision to avoid non-public constructors/setters was motivated by more of a feeling for best-practice rather than by-design.

I envisioned a scenario like this could come up someday so I left open an extension point in Bogus' Faker<T> for this very purpose. The IBinder interface can be used to change the introspection visibility that Faker<T> uses when reflecting over type T. For example, passing in your own custom new Faker<T>(binder: myBinder) with your binding flags can teach Bogus some new tricks about how to locate and discover what MemberInfos are available on type T.

As you can see here. the default Binder uses:

protected internal BindingFlags BindingFlags = BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public;

If you want to change the visibility of what members of T are seen by Faker<T>, you'd want to do something like this:

public class MyBinder : Binder
{
   public override Dictionary<string, MemberInfo> GetMembers(Type t)
   {
      // return a dictionary of <membername, memberinfo> I want to
      // discover over type T
   }
}
var myBinder = new MyBinder()
var faker  = new Faker<T>( binder: myBinder );

There are probably situations where this might not be enough to completely resolve your issue, so I'd be interested in knowing more about your situation and how we could possibly resolve any issues you might have. If you could post some examples of what you're trying to achieve with Bogus that would be super helpful.

Thanks,
Brian

🚗 🚙 "Let the good times roll..."

@waldimen
Copy link

I upgraded to latest version of Bogus and have of my tests failed becaue of the issue, I am using internal properties in many places. I tried to use a non-public binder but it causes other errors on collections.
I have to revert to previous version for now.

@bchavez bchavez added the bug label Sep 27, 2018
bchavez added a commit that referenced this issue Sep 27, 2018
… couldn't find a public setter. This change allows setting of non-public members. Reverts back to previous Bogus behavior which originally allowed setting of non-public members. Related #179.
bchavez added a commit that referenced this issue Sep 27, 2018
… couldn't find a public setter. This change allows setting of non-public members. Reverts back to previous Bogus behavior which originally allowed setting of non-public members. Related #179.
@bchavez
Copy link
Owner

bchavez commented Sep 27, 2018

Hey @waldimen ,

Thanks for letting me know. I took a deeper look at the issues here and it looks like we had a regression in PR #170. Prior to Bogus v23.0.3, we allowed setting of internal non-public members.

I think I've fixed the issue in the recent a3d2dc9 commit and will issue a new release shortly. The next release should restore the previous behavior prior to v23.0.3 and allow the setting of internal members as normal.

I appreciate the bug report! I apologize for the inconvenience.

Thanks,
Brian

🎧 🎶 Antiserum & Mayhem - Trippy

@bchavez
Copy link
Owner

bchavez commented Sep 27, 2018

Hey @waldimen,

Bogus v24.2.0 should be the latest release with the internal property fix.

https://www.nuget.org/packages/Bogus/24.2.0

Please let me know if it resolves your issue.
Brian

🔥 🌋 "We're building it up, to break it back down..."

@mcosta91
Copy link
Author


I think that this fix pretty much solve my problem, actually. I will test it when I go back home tonight.

@bchavez
Copy link
Owner

bchavez commented Sep 27, 2018

Hi Marcus,

Thanks for testing and bringing up the issue. Let me know how it goes. :)

Thanks,
Brian

🏖️ 🎺 Beach Boys - Good Vibrations (Nick Warren bootleg)

@waldimen
Copy link

@bchavez

The new version works perfectly. Thanks for such quick fix :)

@mcosta91
Copy link
Author

Worked like a charm. Thanks 👍

@bchavez
Copy link
Owner

bchavez commented Sep 28, 2018

That's great news. Thank you Marcus and @waldimen for testing. Please don't hesitate to post if there is a blocking issue. I try to resolve issues as quickly as I can. 😺

Thanks,
Brian

💼 👔 "Taking care of business every day... Taking care of business every way..."

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

No branches or pull requests

3 participants