Code cleanup & modernization #31077
Code cleanup & modernization #31077
Conversation
| e => e.GetDeclaredNavigations().Any(n => !n.IsOnDependent && !n.ForeignKey.IsOwnership))) | ||
| e => e.GetDeclaredNavigations().Any(n => n is { IsOnDependent: false, ForeignKey.IsOwnership: false }))) |
There was a problem hiding this comment.
Now see, this is why I hate most of these. It was perfectly terse and clear before.
There was a problem hiding this comment.
I do find the pattern here better - my brain automatically understands that we're checking two things on n, rather than two unrelated conditions (with n being repeated).
But I agree that this is an edge case, do you really hate most of the changes in this PR? IMHO it cuts down on so much crap and makes conditions so much easier to grok... If people hate it we can revert.
There was a problem hiding this comment.
It’s half and half for me. I’m fine doing the occasional bulk cleanup like this. I just put a lot of thought into how I express conditions that it’s sad to see them occasionally mangled in the name of simplification or new language features.
There was a problem hiding this comment.
I probably should have asked/cleared this with the team before doing it - my bad.
We should maybe briefly discuss on Friday - we can also decide to roll at least some of it back, and/or not have this corrected/suggested automatically, but rather decide on a one-by-one basis - though looking at this PR, my personal take is that 95% of these make total sense and do improve code quality.
There was a problem hiding this comment.
This adds a few additional null checks if n and ForeignKey are reference types, could be a reason to not use pattern matching in some cases.
Mostly automated code fixes, especially to apply C# pattern matching.