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

Make Capture.In<>() support System.Collections.Generic.Queue<>() #1198

Closed
Tiefseetauchner opened this issue Aug 9, 2021 · 4 comments
Closed

Comments

@Tiefseetauchner
Copy link

Tiefseetauchner commented Aug 9, 2021

I'm working on some test setups, and Capture.In<>() takes ICollection<> collection as parameter, but Queue only extends ICollection. Queue would be really useful, because it makes it much easier to Validate the parameters in the Verify(), since the Dequeue returns and removes the value, which means no helper method is needed, and Times.Exactly() could yeet as many entries as needed (if that is something that is possible, but it seems like it, from what I read in Mock.Verify(Mock mock, LambdaExpression expression, Times times, string failMessage))

I am not sure if a implementation over ICollection, IEnumerable<> or Queue<> directly would be best, but I shall leave that decision in this contributors capable hands, should they agree with my ascertation that supporting Queue<> would be helpful

(Edit: I am aware of CaptureMatch, but I think supporting Queue directly would be more intuitive, and also there is no documentation on Capture and CaptureMatch that I could find, so it makes it even more confusing when using it. Overall, I think adding it would improve readability and ease of use)

@stakx
Copy link
Contributor

stakx commented Aug 9, 2021

Hi @Tiefseetauchner. ⚓🌊🐚 I understand the use case, but I'm not certain that a new Capture.In overload for Queue<> would be wise... I tend to think it probably wouldn't be. Let me lay out a few arguments:

  • The new method overload could be neither for IEnumerable<> nor for ICollection, since those interfaces only allow us to consume a collection; they don't allow adding new items to it. For the same reason, IReadOnlyCollection<> also isn't a viable candidate.
  • So we'd end up having to create a new overload for Queue<> directly, but that kind of opens a Pandora's box: there are a whole lot of other collection types that still wouldn't be supported; say, Stack<> and collections in System.Collections.Concurrent. Once we begin adding overloads for particular collection types, where will we stop?
  • Capture.In is really a convenience interface around CaptureMatch (as you know). Because it's only for convenience, but not essential, there is no reason for that API to cover everything; it only needs to cover the most likely use cases... and that is in all likelihood List<>. (The fact that the method is typed to ICollection<> instead of List<> is probably because the former could be had for free, so "why not".)

So what this boils down to is whether using Queue<> with Capture.In is something that a lot of people would want to do. (I personally suspect not, but I don't have any data points to prove that.)

For the time being, I would advise you to create e. g. an adapter extension method that wraps an ICollection<>-typed façade around some Queue<> instance, such that you can then plug that queue into Capture.In.

Let's leave this issue open for a while and see what others think. If your request doesn't get much additional support, I'll eventually close this request; if other people want the same thing, we could add overloads for both Queue<> and Stack<> (and perhaps IProducerConsumerCollection<> though I'm not sure about that one).

Do you think this would be a sound approach?

@Tiefseetauchner
Copy link
Author

Hey! That sounds pretty reasonable indeed. Thx for the quick response 👍🏻

@stakx
Copy link
Contributor

stakx commented Aug 22, 2021

Thinking about this some more, I wonder whether it would make sense to make the following additions instead:

partial static class Capture
{
    public static T With<T>(Action<T> callback)
        => Capture.With(new CaptureMatch<T>(callback));

    public static T With<T>(Action<T> callback, Expression<Func<T, bool>> predicate)
        => Capture.With(new CaptureMatch<T>(callback, predicate));
}

This would make it a little easier to use any kind of collection:

  •  Capture.With(list.Add)
  •  Capture.With(stack.Push)
  •  Capture.With(queue.Enqueue)

and so on.

It seems a little superfluous to me, but if we do end up doing anything about Capture, it should probably be this, instead of making collection-specific patches.

P.S.: Unfortunately, the three examples shown above don't work in practice; the C# compiler cannot infer the generic type argument automatically, so one would have to explicitly write e.g. Capture.With<TArg>(queue.Enqueue). Not quite as terse as I had hoped, so these overloads may not be worth adding to the public API after all.

@stakx
Copy link
Contributor

stakx commented May 12, 2022

This feature request hasn't seen any more upvotes nor other activity, so as per my original announcement above, I'm closing this. I'll mark the issue as "unresolved" though so we can more easily find it, should anyone want to pick this up in the future.

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

2 participants