Fix a bug where ToHashSet() is implemented in .Net 4.7.1+ System.Linq, causing failure to compile

[b9chris]

Fix a bug where:
using System.Linq;
using MoreLINQ;

Causes calls to .ToHashSet() to fail to compile, because an identical implementation exists in .Net 4.7.1+

[Orace]

MoreLINQ/MoreLinq/ToHashSet.cs

Lines 17 to 19 in b839848

	#if !NET471
	namespace MoreLinq

This test is at compile time, not at use time.
You can compile with 4.7 and still have the issue.

Maybe you can look what as been done in similar case (like Zip).

Anyway to fix it at user level, the README file is pretty clear about it:

https://github.com/morelinq/MoreLINQ#usage

[b9chris]

This test is at compile time, not at use time.

Not sure what you mean by this. This flag is meant to prevent inclusion of the ToHashSet() extension method in 4.7.1+, and that's what it does. I've tested it - it fixes the issue.

You can compile with 4.7 and still have the issue.

I think you mean it was introduced in 4.7 not 4.7.1? Simple enough to change, I'll fix that.

Maybe you can look what as been done in similar case (like Zip).

Anyway to fix it at user level, the README file is pretty clear about it:

https://github.com/morelinq/MoreLINQ#usage

What's documented there is a workaround, not a fix. What I'm proposing is a fix.

[Orace]

This test is at compile time, not at use time.

Not sure what you mean by this. This flag is meant to prevent inclusion of the ToHashSet() extension method in 4.7.1+, and that's what it does. I've tested it - it fixes the issue.

You can compile with 4.7 and still have the issue.

I think you mean it was introduced in 4.7 not 4.7.1? Simple enough to change, I'll fix that.

The '#if' tests are done at compile time.
MoreLinq is almost distributed via nuget package that are pre-compiled.

Actually I don't know wish version of the framework is used to build the package.

Let say that the package is built in framework 4.5 (or anything not 4.7.1, like 4.7).
At compile time "NET471" will be false and the '#if' will be ignored and ToHashSet will be put in the package:

• users of framework 4.5 will find one ToHashSet method (from MoreLinq).
• user of framework 4.7.1 will find two ToHashSet method (MoreLinq and System.Linq).

Now, let say that the package is built in framework 4.7.1
At compile time "NET471" will be true and the '#if' will trigger, ToHashSet will not be put in the package:

• users of framework 4.5 will find zero ToHashSet method.
• users of framework 4.7.1 will find one ToHashSet method (System.Linq).

Hence, for nuget package distribution, nothing is fixed here.

Since the final user framework version is unknown, and because we don't want to remove any functionality for users with a framework version prior to 4.7.1, and because we don't want a MoreLinq for 3.5, one for 4.5, one for 4.7.1, etc... a workaround is advised (using alias, or static using).

Hope it's clear enough, and sorry to no be clear enough at first try.

[b9chris]

Actually I don't know wish version of the framework is used to build the package.

I see, I think you just need to read up on multi-framework targeting. More than one framework is used in multi-target frameworks. MoreLINQ is multi-target. So, the answer is - multiple frameworks are used, and for the 4.7.1 build in this case, the flag is true. For others, like the 4.5.1 build, the flag is false. It sounds like you've declared this won't work without actually testing anything, so I'd prefer you try it before issuing further objections. https://docs.microsoft.com/en-us/dotnet/standard/library-guidance/cross-platform-targeting

[viceroypenguin]

I have wondered why this solution isn't used before. I agree with @b9chris, MoreLINQ should be able to block off certain API based on which version it is compiled with. This will make the provided work-around unnecessary.

This could also be applied to .Append() and .Prepend(), provided in the latest .net core offerings.

[Orace]

I have wondered why this solution isn't used before

The cons I see :

• Hard to maintain.
• Many more target frameworks.
• Need a quick new release for any method integrated in a version of a framework (hopefully method signatures matched for now).

This will make the provided work-around unnecessary.

The workaround is still useful in case of name collision with a third-party library.

[viceroypenguin]

On the other hand, the workaround is extra work in the case where the consumer is targeting multiple frameworks. As it is, I have multiple places where I have to include:

#if NETFX
using static MoreLinq.Extensions.AppendExtension;
#endif

If MoreLINQ handled this directly, it would be a simple using MoreLinq; in those cases.

However, I do get your point about extra work in the case of a new .net core version; they are continually adding new Enumerable features that overlap MoreLINQ.

[viceroypenguin]

@b9chris Please check out the morelinq.temp package as a proof of concept for a solution for this issue, built off of https://github.com/viceroypenguin/MoreLINQ/tree/multi-framework-support branch. If you have any comments, put them in #749.

[atifaziz]

This has been superseded by PR #945.