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

Adding method to set public property setters after object is constructed #2

Open
georgiosd opened this issue Aug 9, 2013 · 9 comments

Comments

@georgiosd
Copy link

Robert! Looks like you have a knack for testing frameworks (I've made some fuss about Seleno) :)

Say, isn't it quite redundant to manually apply the lambda expressions to the test object in BuildObject() - any reason why you didn't just loop over them and set them yourself?

@georgiosd
Copy link
Author

Here's what I mean:

public abstract class ExtendedTestDataBuilder<TObject, TBuilder> : ITestDataBuilder<TObject>
        where TObject : class
        where TBuilder : class, ITestDataBuilder<TObject>
    {
        class Entry
        {
            public MemberExpression MemberExpression;
            public Action<TObject, object> Set;
            public object Value;
        }

        private readonly List<Entry> properties = new List<Entry>();
        private ProxyBuilder<TObject> proxyBuilder;

        public TObject Build()
        {
            if (proxyBuilder == null)
                return BuildObject();
            TObject proxy = proxyBuilder.Build();
            AlterProxy(proxy);
            return proxy;
        }

        protected abstract TObject BuildObject();

        protected TObject FillObject(TObject obj)
        {
            foreach (var entry in properties)
                entry.Set(obj, entry.Value);
            return obj;
        }

        public TBuilder AsProxy()
        {
            proxyBuilder = new ProxyBuilder<TObject>(properties
                .ToDictionary(p => p.MemberExpression.Member.Name, p => p.Value));
            return this as TBuilder;
        }

        protected virtual void AlterProxy(TObject proxy)
        {
        }

        public void Set<TValue>(Expression<Func<TObject, TValue>> property, TValue value)
        {
            properties.Add(new Entry
            {
                MemberExpression = (MemberExpression)property.Body,
                Set = ExpressionBuilder.CreateSetter(ExpressionBuilder.GeneralizeExpression(property)),
                Value = value
            });
        }

        public TValue Get<TValue>(Expression<Func<TObject, TValue>> property)
        {
            if (!Has(property))
                throw new ArgumentException(
                    string.Format("No value has been recorded yet for {0}; consider using Has(x => x.{0}) to check for a value first.",
                                  ExpressionBuilder.GetName(property)), "property");
            return (TValue)properties.Single(p => p.MemberExpression.Equals(property.Body)).Value;
        }

        public TValue GetOrDefault<TValue>(Expression<Func<TObject, TValue>> property)
        {
            if (!Has(property))
                return default(TValue);
            return Get(property);
        }

        public static IListBuilder<TBuilder> CreateListOfSize(int size)
        {
            return Builder<TBuilder>.CreateListOfSize(size);
        }

        protected bool Has<TValue>(Expression<Func<TObject, TValue>> property)
        {
            return properties.Any(p => p.MemberExpression == (MemberExpression)property.Body);
        }
    }

Usage:

protected override Foo BuildObject()
        {
            return FillObject(new Foo()); // keeping BuildObject in case there is no public default ctor - but we can also give that override behavior
        }

@georgiosd
Copy link
Author

If you like it, I can send you a pull request later.

@robdmoore
Copy link
Member

Hi mate - good to hear from you!

Thanks for the suggestion :)

In this case it's actually a deliberate design choice that you reference the properties explicitly when initialising the object. This framework was explicitly designed for creating domain models with protected setters on properties and constructors that enforce domain invariants. By forcing you to use the constructor it ensures you are meeting all domain invariants of the objects generated for your test and the test code is performing tests using objects that can exist in production (rather than objects that are potentially inconsistent).

If you have objects that don't conform to that then either you will have public setters (in which case newing up the objects with the setters being set at the same time is just as terse) or you can easily use NBuilder to do it. Alternatively, the .AsProxy() functionality in NTestDataBuilder will set .Returns on all public properties based on the values you have set and achieves the same result (except of course all virtual methods will be mocked).

If there are other scenarios that NTestDataBuilder accommodates that you think NBuilder doesn't, but it would require this change, or other changes I'm all ears :)

@georgiosd
Copy link
Author

:)

Hm... it doesn't break the domain invariance - you still have to call the ctor yourself - it's just that properties are set automatically, without creating a proxy. You could run a check for non public setters and throw to avoid use private setters.

I guess it's not worth doing if AsProxy() does the same job, though I'm not sure how it would affect calculated properties etc.

@robdmoore
Copy link
Member

You can break invariance if you set a property that doesn't have a public setter. Most of the classes we create have protected setters which is the viewpoint I was coming from.

If you see a use for the library and you have public property setters and would find that method useful I'm more than happy to put it in, I would just want to change it to only set public setters.

AsProxy would work for calculated properties if they aren't virtual, ortherwise you would need to set what value they should be.

@georgiosd
Copy link
Author

Whatever you think, it's your project after all - I'm happy just implementing this behavior on my project! And yes, limiting to public setters makes sense

Date: Sat, 10 Aug 2013 01:59:35 -0700
From: notifications@github.com
To: NTestDataBuilder@noreply.github.com
CC: georgiosd@live.com
Subject: Re: [NTestDataBuilder] Automating BuildObject() (#2)

You can break invariance if you set a property that doesn't have a public setter. Most of the classes we create have protected setters which is the viewpoint I was coming from.

If you are a use for the library and you have public property setters and would find that method useful I'm more than happy to put it in, I would just want to change it to only set public setters.

AsProxy would work for calculated properties if they aren't virtual, ortherwise you would need to set what value they should be.


Reply to this email directly or view it on GitHub.

@robdmoore
Copy link
Member

Cool. While it's an easy change to make it's not high on my priorities at the moment - there are a few other projects that have more important changes I have lined up.

If this becomes important for you though feel free to ping back and I'll make some time to add it in.

@robdmoore robdmoore reopened this Aug 12, 2013
@georgiosd
Copy link
Author

No rush, you've done well and made an interface for this so I can customize to my heart's content :)

@cottsak
Copy link

cottsak commented Sep 28, 2016

What is actually up-for-grabs on this one? I thought @robdmoore made it clear in #2 (comment) that the current implementation of preventing setting of protected/private set members was a design choice.

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