-
Notifications
You must be signed in to change notification settings - Fork 2
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
Create duplicate invariant key validator #184
Conversation
//this can be expanded with other validate functionality | ||
evidence.AddRange(validateInvariantUniqueness(input, state)); | ||
|
||
return ResultReport.FromEvidence(evidence); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to create a list:
var evidence = validateInvariantUniqueness(input,state);
return ResultReport.FromEvidence(evidence);
If there are multiple steps in the future, you can just do
var evidence = validateInvariantUniqueness(input,state)
.Concat(validateOtherInvariant(input,state));
.Distinct())) //Distinct to remove paths that are encountered multiple times per invariant | ||
.Where(kv => kv.Paths.Count() > 1); //Remove entries that only have a single path. These are not incorrect. | ||
|
||
return PathsPerDuplicateInvariantKey.Select(c => new IssueAssertion(Issue.PROFILE_ELEMENTDEF_INCORRECT, $"Duplicate key '{c.Key}' in paths: {string.Join(", ", c.Paths)}")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would, because in the end this enumeration will need to be fully evaluated, call ToList() on the end-result. This way, callers of our functions can never shoot themselves in the foot by enumerating this (expensive) LINQ statement multiple times by accident. Also, it makes the debugging more clear, since, if we don't do the ToList()), you will only get an exception deep inside the ResultReport.FromEvidence()
(that's the function finally triggering the evaluation of the IEnumerable) - and you will have no clue what's going on!
BTW: Nice LINQ query ;-) |
//this can be expanded with other validate functionality | ||
evidence.AddRange(validateInvariantUniqueness(input, state)); | ||
var evidence = validateInvariantUniqueness(input, state).ToList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your new function already returns a list now, so, this should not be necessary anymore!
fixes #143