Skip to content

Conversation

@adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Nov 15, 2022

fixes #1971

@adamsitnik adamsitnik marked this pull request as ready for review November 16, 2022 13:15
@adamsitnik adamsitnik merged commit 350a618 into dotnet:main Nov 17, 2022
@adamsitnik adamsitnik deleted the genericFactory branch November 17, 2022 09:29
throw new ArgumentNullException(nameof(defaultValueFactory));
}

SetDefaultValueFactory(() => defaultValueFactory());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you delete this null check? ArgumentNullException is still documented but the method no longer throws it. A null argument will instead be saved in the closure and cause NullReferenceException if the factory is called later.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment applies to the public Argument(Func<T> defaultValueFactory) constructor.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All default value factories should be generic

3 participants