0

While porting my (large) application from .NET Framework 4.X to .NET 5+, stumbled over the new System.Collections.Generic.CollectionExtensions, which did not exist in the .NET Framework. Before it existed, I created an extension method GetValueOrDefault, which has a similar signature as the new one - but with the difference that I check for null keys and returned the default value in that case instead of throwing an exception.

In some cases, I got an ambigous match - however not always (presumably because my method has a default parameter for the value). Now, the .NET version is used most of the time and my application crashes everywhere...

Is there any way to globally hide the extension method or to always prefer my version?

PS: I know there exists a similar question (Ambiguous extension method), but it is already quite old, dating before .NET core

Regarding Renaming: Actually, I don't want to rename my method.

  1. It existed before .NET Core
  2. When I rename it, I may accidentally call the .NET version (I am used to this name for years), which may throw an exception when I don't expect it. I don't see any real use case when it would be useful for this method to throw instead of returning the default value.
LionAM
  • 1,271
  • 10
  • 28
  • 5
    The refactoring tools in VS are pretty simple(and working) and it should be the easiest to just rename your method. – Ralf Aug 22 '23 at 13:01
  • 2
    The old answer is valid. This isn't about .NET Core (.NET 5+ is .NET *Core* 5+). If you have two methods with identical signatures, you need to ensure that only one is visible in a file. That means only including one of them with `using`, or aliasing one of them – Panagiotis Kanavos Aug 22 '23 at 13:01
  • @Ralf A simple "search and replace" won't do, as there are other methods with the same name (e.g. on different interfaces or even on the Nullable type). As the port is mostly done, I also cannot simply rename my method, as it is not used any more. And we are talking about hundreds of files where it was used – LionAM Aug 22 '23 at 13:09
  • 2
    Your extension method breaks the guarantees of an` IDictionary<>` so even after aliasing, your code will still surprise people, including *you* after a while. It's better to use a Rename refactoring to change your method's name to something else. You can then change the method to either return a default value for null keys or delegate to `GetValueOrDefault` – Panagiotis Kanavos Aug 22 '23 at 13:09
  • Refactoring isn't Search&Replace. Refactoring is smarter than that. Mark you method in the editor and hit F2. – Ralf Aug 22 '23 at 13:11
  • 1
    @LionAM the `Rename` refactoring will only change the method you run it on. As for the rest, you shouldn't have let the port to proceed that much before making this change. That's why Jon Skeet warns against "subtle changes" - if you delay fixing a problem it becomes a lot more expensive to fix – Panagiotis Kanavos Aug 22 '23 at 13:11
  • @Panagiotis Kanavos I was not aware of this problem before the application did start again after the refactoring... – LionAM Aug 22 '23 at 13:13
  • @LionAM `it is not used any more` the compiler still sees the usage, which is why it throws compilation errors. You can still find the usages, even if replacing them isn't as easy – Panagiotis Kanavos Aug 22 '23 at 13:14
  • @Panagiotis Kanavos There are no compiler errors, but runtime errors. – LionAM Aug 22 '23 at 13:15
  • Your question said `I got an ambigous match`. Post your code instead of describing it. If some matches are missed due to default parameter values, remove the default so you can get more compilation errors. – Panagiotis Kanavos Aug 22 '23 at 13:17
  • In any case, either this get closed as a duplicate of the older question, or you post enough code so people can reproduce the issue and help in *your* specific case. You don't need to post a lot of code. One call where the ambiguous match is detected and one where it isn't, along with your method's signature. The code effectively becomes a test case, with "success" failing on all calls. Once you have that, you could write an analyzer/fixer that replaced the calls with a new name. – Panagiotis Kanavos Aug 22 '23 at 13:21
  • In fact, you could probably write an analyzer that looks for all calls to `GetOrDefault(IDictionary...)` and a fixer that replaced them with your own method. – Panagiotis Kanavos Aug 22 '23 at 13:24

1 Answers1

6

I would say the simplest fix here is to rename your extension method (e.g. NullKeySafeGetValueOrDefault), and change your code everywhere.

Even if there's a subtle way of getting the compiler to prefer your version, it's much better to be unsubtle about it. Anywhere someone is reading the code, they should be left in no doubt at all which method is going to be called.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • A simple "search and replace" won't do, as there are other methods with the same name (e.g. on different interfaces or even on the Nullable type). When I rename the extension method, it wont be updated in the code, as most of the time the .NET version is linked now... – LionAM Aug 22 '23 at 13:11
  • Maybe try renaming it in your existing code _before_ porting it? – Martin Costello Aug 22 '23 at 13:24
  • @Martin Costello: Too late - I already invested a week to get it compile again and to allow starting the application – LionAM Aug 22 '23 at 13:29
  • I didn't say it would be trivial to do - but I still think it's the best option. – Jon Skeet Aug 22 '23 at 13:41
  • 2
    @LionAM - then you've just learnt the value of having a source control system where you can retroactively create a branch from the point before you started the upgrade, do the suggested rename and then merge forwards... – Damien_The_Unbeliever Aug 22 '23 at 13:42
  • @Damien_The_Unbeliever While this would maybe actually solve the problem of renaming, it wont stop me and my colleagues from accidentally using the .NET version, not expecting it to throw (as my version did never throw). – LionAM Aug 22 '23 at 13:53
  • 2
    @LionAM: Trying to get out of that habit/mindset *as early as possible* is wise - otherwise when you move to some *other* codebase which doesn't have your extension method, you'll be in the same situation but with more time reinforcing the problem. – Jon Skeet Aug 22 '23 at 13:56
  • @Jon Skeet I see your point. But I fear I would in future still accidentally type `GetValueOrDefault` as I'm used to it for more than 10 years... When I had some other code base, the function did either not exist or I implemented it. – LionAM Aug 22 '23 at 14:23
  • 4
    @LionAM: Well, *every* code base you use in the future is going to have the version from the framework. And every newcomer to your codebase is at least *quite* likely to expect the framework behavior, too... this is really just *not* a good position long term, so it's best to try to fix it ASAP in my view. – Jon Skeet Aug 22 '23 at 14:54