Resolve Dummy Lazy values lazily

[TimLovellSmith]

I thought of this while reviewing docs for dummies and thinking about how many dummy types/objects might get created.

Wouldn't it be neat if this passed, and in fact a dummy type for WantsToBeDummyed was not actually generated, if Lazy.Value was never called? [A definite possibility when faking objects with 'lazy dependencies].

namespace DoesntMatter
{
    public class WantsToBeDummyed
    {
        public static bool ynstanshyated;

        public WantsToBeDummyed()
        {
            ynstanshyated = true;
        }

        public virtual long CanOverryde()
        {
            return 3L;
        }
    }             

    class Program
    {
        static void Main(string[] args)
        {
            Assert.False(WantsToBeDummyed.ynstanshyated);
            var dummy = A.Dummy<Lazy<WantsToBeDummyed>>();
            Assert.False(WantsToBeDummyed.ynstanshyated); // what, its true?!

            var ynnerDummy = dummy.Value;
            Assert.True(WantsToBeDummyed.ynstanshyated);
        }
    }
}

[thomaslevesque]

Interesting... I was about to say "but they are lazy", because IsValueCreated returns false... But in fact the dummy is created immediately. Basically, what we do is this:

var dummyValue = A.Dummy<WantsToBeDummyed>();
return new Lazy<WantsToBeDummyed>(() => dummyValue);

I guess we could make them really lazy. But what would be the benefit? Maybe a small performance improvement, but I doubt it would make a big difference.

[TimLovellSmith]

The real reason I filed this issue is not the scenario above. The real reason I want it to work this way is because of the test scenarios where .Value is never actually called. (And sometimes there are evil types which do too much stuff in their constructor. I know, would be great to fix those, sigh...)

class HasDependencies {
    Lazy<T1> Dependency1 { get; set; }     
    Lazy<T2> Dependency2 { get; set; }
}

A.CallTo(() => myHasDependencies.Dependency1.Value.DoSomething()).Returns("wow, that was fast!");

[thomaslevesque]

The real reason I want it to work this way is because of the test scenarios where .Value is never actually called

What's the problem with these scenarios? Even if what is returned by .Value is created in advance, the Lazy<T> won't know it, and will still have IsValueCreated == false if you don't access .Value.

Unfortunately, in the example you posted, myHasDependencies.Dependency1.Value will be evaluated when you configure the call, and there isn't much we can do about it... We need to find the fake on which the DoSomething() method is called, so we evaluate that part of the expression. And anyway, you can't configure a fake that hasn't yet been created!

[TimLovellSmith]

I mean I'm bringing this up primarily motivated by efficiency, not correctness, whatever that is. Less electrons moved, less time running tests, less carbon emitted, all that good stuff.

So from my particular point of view, its fine for .Value to be evaluated when configuring the call, since we're probably going to later actually do the call anyway... and at that point, .Value is going to get called again and return the cached value. So for me, this is really all about Dependency2. And my original post, while illustrating the providing behavior, doesn't act as the clearest requirements doc!

[blairconrad]

A good goal, @TimLovellSmith. Truth be told, I think your version may be superior. If we could be sure there's no downside to changing the behaviour, I think I'd be advocating for the change. But we've been promising to return a Lazy that already has a value, and I worry that the change might break people's tests.

[blairconrad]

… and I'm a little afraid that if we were to make the change, I'd be tempted to implement by passing a Fake<Func<T>> to the Lazy constructor, and in a lot of cases, that might be more work than calling the T constructor!

[thomaslevesque]

But we've been promising to return a Lazy that already has a value

Have we? In How are the dummies made, we just say this:

If T is a Lazy<TValue> the returned Dummy will be an actual Lazy<TValue> whose Value is a Dummy of type TValue, or a default TValue if no Dummy can be made for TValue.

To me, it doesn't seem to imply that the value is created eagerly.

and I worry that the change might break people's tests.

I don't think it could. The value can only be accessed via the Value property anyway, which will execute the creation callback if it's the first access. Whether the callback returns a value that was already created, or creates a new one, doesn't really matter and shouldn't have any visible effect.

… and I'm a little afraid that if we were to make the change, I'd be tempted to implement by passing a Fake<Func<T>> to the Lazy constructor, and in a lot of cases, that might be more work than calling the T constructor!

That's a clever approach, I would never have thought of it! But yeah, it might not be the most efficient, especially with the way we create fake delegates, which is quite slow.

[blairconrad]

If T is a Lazy the returned Dummy will be an actual Lazy whose Value is a Dummy of type TValue, or a default TValue if no Dummy can be made for TValue.

To me, it doesn't seem to imply that the value is created eagerly.

Ah. Okay, then. Maybe I've been reading it wrong!

I don't think it could. The value can only be accessed via the Value property anyway, which will execute the creation callback if it's the first access. Whether the callback returns a value that was already created, or creates a new one, doesn't really matter and shouldn't have any visible effect.

IsValueCreated's initial setting would change. But maybe it's not such a big deal.

[blairconrad]

I've been thinking about IsValueCreated changing, and it makes me a little sad, partly because it's a change, and partly because it means we'll be introducing deviation relative the Taskish Dummies, whom we're creating as completed tasks. The analogue seems to me to be a Lazy that already has a value.

[thomaslevesque]

IsValueCreated's initial setting would change

How so? Currently when we create a dummy Lazy<T>, its IsValueCreated property returns false. So the fact that the dummy T is already created isn't visible, so if we actually create the dummy T lazily, it will make no difference from the user's point of view. Just less work if .Value is never accessed.

[thomaslevesque]

and partly because it means we'll be introducing deviation relative the Taskish Dummies, whom we're creating as completed tasks. The analogue seems to me to be a Lazy that already has a value.

That's an interesting point. I didn't consider a similarity between Task and Lazy. They can be viewed as similar, I guess, but they're quite different beasts. When you create a Task<T> using Task.Run or by calling an async method, the task is usually already running (I don't think anyone still uses "cold" tasks that you need to Start explicitly), and possibly completed. When you create a Lazy<T>, the work to create the T is explicitly not started; it will only be done when you access .Value.

[TimLovellSmith]

The main use for 'IsValueCreated' that I know about is checking whether you really created something so you know whether you have to Dispose it, without accidentally creating one in the process!

IMHO if they wrote any code that cares enough to test a boolean value, like this, which is really designed to support scenarios where you don't want to be sure that Value was created shouldn't callers also care enough to handles the 'it can return false' case? Otherwise they should just not even have been testing IsValueCreated... :)

Yes my unit test where I use IsValueCreated is somewhat a counterexample, but this is not what I consider a typical use case! [And I do handle the return false value, by failing/passing the test according to my desire. :-) ]

[blairconrad]

1. Thanks, @thomaslevesque. I'd misremembered how we created Lazys. You're right, of course.
2. And as a result, my Task blathering should be ignored!

I'm okay with changing creation to be lazy.
At first I was concerned about how this would change the type resolution graph loop-detection (if at all). I think it'll be okay. If there's a loop, we'll currently use default(T) for the Value.
With lazy resolution, this check wouldn't happen until the Value is accessed, and the LoopDetectingResolutionContext will be empty, if if I'm right. But even so, if there was originally going to be a loop, it should show up in this case and then we'll get the same behaviour. Make sense?

[thomaslevesque]

Make sense?

I think so...

But as you said, we won't be in a loop when the value is actually resolved. We just need to make sure to use a new LoopDetectingResolutionContext in the creation callback... If we capture the original one, it's likely to cause trouble.

[blairconrad]

If we capture the original one, it's likely to cause trouble.

I think it'll be empty by the time the Value is accessed, actually. But a new one is safer.

[TimLovellSmith]

If you have a lazy, it will be harder to have a loop, because a loop should be taking you back to Lazy, and then nobody will call Value... unless they are doing it, unlazily, as part of initializing something. In which case yes, you might get a loop. But if its that kind of loop, wouldn't it all happen within one loop detecting context already? (Not really sure, trying to convince myself...)

[blairconrad]

I think we're unlikely to have a loop within the Lazy resolution, but we recently fixed a bug when a class's constructor took an instance of the class, so weird things do come up. I don't think the proposed change will introduce any problems on that front.

[blairconrad]

Labelling as breaking because there's a remote chance that clients are expecting a side effect from the constructor to execute at a particular time, as @thomaslevesque noted in private chat.

[afakebot]

This change has been released as part of FakeItEasy 6.0.0-beta.1.

[thomaslevesque]

Thanks for suggesting this @TimLovellSmith! Look for your name in the release notes! 🥇

[TimLovellSmith]

Woohoo! 🎉
Going to give it a spin!