{"action":"created","issue":{"url":"https://api.github.com/repos/nsubstitute/NSubstitute/issues/401","repository_url":"https://api.github.com/repos/nsubstitute/NSubstitute","labels_url":"https://api.github.com/repos/nsubstitute/NSubstitute/issues/401/labels{/name}","comments_url":"https://api.github.com/repos/nsubstitute/NSubstitute/issues/401/comments","events_url":"https://api.github.com/repos/nsubstitute/NSubstitute/issues/401/events","html_url":"https://github.com/nsubstitute/NSubstitute/issues/401","id":321084837,"number":401,"title":"Add support for the `out` and `ref` parameters matchers","user":{"login":"zvirja","id":1074182,"avatar_url":"https://avatars1.githubusercontent.com/u/1074182?v=4","gravatar_id":"","url":"https://api.github.com/users/zvirja","html_url":"https://github.com/zvirja","followers_url":"https://api.github.com/users/zvirja/followers","following_url":"https://api.github.com/users/zvirja/following{/other_user}","gists_url":"https://api.github.com/users/zvirja/gists{/gist_id}","starred_url":"https://api.github.com/users/zvirja/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zvirja/subscriptions","organizations_url":"https://api.github.com/users/zvirja/orgs","repos_url":"https://api.github.com/users/zvirja/repos","events_url":"https://api.github.com/users/zvirja/events{/privacy}","received_events_url":"https://api.github.com/users/zvirja/received_events","type":"User","site_admin":false},"labels":[{"id":712764818,"url":"https://api.github.com/repos/nsubstitute/NSubstitute/labels/feature-request","name":"feature-request","color":"bfd4f2","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/nsubstitute/NSubstitute/milestones/5","html_url":"https://github.com/nsubstitute/NSubstitute/milestone/5","labels_url":"https://api.github.com/repos/nsubstitute/NSubstitute/milestones/5/labels","id":3280136,"number":5,"title":"v4.0","description":null,"creator":{"login":"dtchepak","id":70105,"avatar_url":"https://avatars3.githubusercontent.com/u/70105?v=4","gravatar_id":"","url":"https://api.github.com/users/dtchepak","html_url":"https://github.com/dtchepak","followers_url":"https://api.github.com/users/dtchepak/followers","following_url":"https://api.github.com/users/dtchepak/following{/other_user}","gists_url":"https://api.github.com/users/dtchepak/gists{/gist_id}","starred_url":"https://api.github.com/users/dtchepak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dtchepak/subscriptions","organizations_url":"https://api.github.com/users/dtchepak/orgs","repos_url":"https://api.github.com/users/dtchepak/repos","events_url":"https://api.github.com/users/dtchepak/events{/privacy}","received_events_url":"https://api.github.com/users/dtchepak/received_events","type":"User","site_admin":false},"open_issues":5,"closed_issues":3,"state":"open","created_at":"2018-04-19T05:25:48Z","updated_at":"2018-05-08T08:12:16Z","due_on":null,"closed_at":null},"comments":11,"created_at":"2018-05-08T08:10:30Z","updated_at":"2018-05-14T07:34:56Z","closed_at":null,"author_association":"COLLABORATOR","body":"Currently we don't provide a way to specify the `Arg.Any()` matcher for the `ref` and `out` arguments, while internally we support them. Let's discuss a way how we could support them\r\n\r\n------\r\nI see two ways to support `ref/out` arguments.\r\n\r\n### Way 1. Create extra methods.\r\n\r\nWe can create methods like:\r\n- `ref T Arg.AnyRef<T>()`\r\n- `ref T Arg.IsRef(42)`\r\n- `ref T Arg.IsRef(x => x == 42)`\r\n\r\nAnd usage:\r\n```c#\r\nsubstitute.OutMethod(out Arg.AnyRef<int>()).Returns(xxx);\r\nsubstitute.RefMethod(ref Arg.IsRef<int>(x => x == 42)).Returns(xxx);\r\n```\r\n\r\nThe cons is that we will now have a set of extra-methods for the `ByRef` parameters.\r\n\r\n### Way 2. Return `ref T` for the existing methods\r\n\r\nAnother interesting way would be to change signature for all the existing `Arg.*` methods and return `ref T` instead of `T`. Later result might be used for both `ByRef` and regular parameters:\r\n\r\n```c#\r\n// Method inside Arg class.\r\npublic static ref T Any<T>()\r\n{\r\n    var argValue = ArgSpecQueue.EnqueueSpecFor<T>(new AnyArgumentMatcher(typeof(T)));\r\n    return ref new DummyValueContainer<T>(argValue).Value;\r\n}\r\n\r\nprivate class DummyValueContainer<T>\r\n{\r\n    public T Value;\r\n\r\n    public DummyValueContainer(T value)\r\n    {\r\n        Value = value;\r\n    }\r\n}\r\n\r\n// USAGE\r\n\r\nsubstitute.RegularMethod(Arg.Any<T>());\r\nsubstitute.RefMethod(ref Arg.Any<T>());\r\nsubstitute.OutMethod(out Arg.Any<T>());\r\nsubstitute.InMethod(in Arg.Any<T>());\r\n//or\r\nsubstitute.InMethod(Arg.Any<T>());\r\n```\r\n\r\nIt looks awesome as for me. It will have a bit of performance overhead (make a reference, create an extra-object), but we could measure whether it's significant or not. Also it's cool that `ref` value could be easily converted to non-ref one, so `ref` result works perfect for non-ref methods.\r\n\r\nI'd proceed with the latter one (because of its beauty and simplicity), but there are a few things to consider:\r\n- it's a change of the signature. We are shipping a major version, so it might be completely fine.\r\n- probably, clients will need to use C# 7 to compile the project.\r\n- a bit of performance overhead (should be measured).\r\n\r\n@dtchepak @alexandrnikitin Your thoughts? 😉 \r\n"},"comment":{"url":"https://api.github.com/repos/nsubstitute/NSubstitute/issues/comments/388723784","html_url":"https://github.com/nsubstitute/NSubstitute/issues/401#issuecomment-388723784","issue_url":"https://api.github.com/repos/nsubstitute/NSubstitute/issues/401","id":388723784,"user":{"login":"alexandrnikitin","id":1798438,"avatar_url":"https://avatars0.githubusercontent.com/u/1798438?v=4","gravatar_id":"","url":"https://api.github.com/users/alexandrnikitin","html_url":"https://github.com/alexandrnikitin","followers_url":"https://api.github.com/users/alexandrnikitin/followers","following_url":"https://api.github.com/users/alexandrnikitin/following{/other_user}","gists_url":"https://api.github.com/users/alexandrnikitin/gists{/gist_id}","starred_url":"https://api.github.com/users/alexandrnikitin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexandrnikitin/subscriptions","organizations_url":"https://api.github.com/users/alexandrnikitin/orgs","repos_url":"https://api.github.com/users/alexandrnikitin/repos","events_url":"https://api.github.com/users/alexandrnikitin/events{/privacy}","received_events_url":"https://api.github.com/users/alexandrnikitin/received_events","type":"User","site_admin":false},"created_at":"2018-05-14T07:34:56Z","updated_at":"2018-05-14T07:34:56Z","author_association":"OWNER","body":"I'll share my thoughts too ;) The second way looks more appealing, the syntax is nice and clear. I would go with it. I believe we should support pre-C#7 versions. (We did that mistake with migrating to .NET Standard 1.3 only, while the intention was good, people still want to use latest versions of libraries with older frameworks.) While the global Find/Replace for `ArgC` is an easy refactoring, the dedicated namespace with the same `Arg.Is/Arg.Any/...` looks cleaner for me. I wonder if this can be done via Roslyn analyzer/refactoring. Another option, in case of a feature release, is to go with a dedicated namespace for the new signature. It can be deprecated in few years when we fully migrate to C#7."}}
{"actor":{"display_login":"alexandrnikitin"}}