Fix for no value if [Option] arg equals to [Value] arg #401
Conversation
Make comparisons by reference in TokenPartitioner.
| var values = nonOptions.Where(v => v.IsValue()).Memorize(); | ||
| var errors = nonOptions.Except(values).Memorize(); | ||
| var errors = nonOptions.Except(values, ReferenceEqualityComparer.Default).Cast<Token>().Memorize(); |
There was a problem hiding this comment.
Are you casting back to Token because the comparer casts everything into Object? What if you made the comparer inherit IEqualityComparer<Token> and then called [Object.ReferenceEquals](https://msdn.microsoft.com/en-us/library/system.object.referenceequals(v=vs.110\).aspx) in the implementation rather than ==? That's a bit more explicit too, and wouldn't need a comment explaining that we're aiming for reference equality on line 16.
There was a problem hiding this comment.
I realized that I can make other cast - (IEqualityComparer<Token>)ReferenceEqualityComparer.Default so Cast<Token> will not be needed. Or can make modify comparer to classic generic version - ReferenceEqualityComparer<T>. Don't want to inherit IEqualityComparer<Token>, it's too concrete for this task.
Will replace == with ReferenceEquals.
There was a problem hiding this comment.
Generic sounds good to me. Any time we can make the type system work for us is a win in my book 😃
There was a problem hiding this comment.
But reference comparer doesn't have to be generic because it compares references anyway. I used this implementation from stackoverflow. As author says it's using contravariance feature so we can cast comparer to any type of IEqualityComparer<T> and we have one instance of comparer for any object type.
|
If you really want |
|
Looks good |
|
I accidently removed FactAttribute from last test with merge. Can you restore it? |
|
Fixed |
Make comparisons by reference in TokenPartitioner. Fixes #398.