Implement FallbackIfEmpty and drop SingleOrFallback

[fsateler]

This would be a breaking change (and thus should be considered before releasing 2.0). Currently there is SingleOrFallback method that takes a delegate. However, I think for consistency with the LINQ API this method should be dropped, and instead provide a FallbackIfEmpty method akin to DefaultIfEmpty, and then use Single, First, Last as needed.

I volunteer to provide the method with tests if you like this approach.

Related to #12 and #67

[atifaziz]

@fsateler I like this approach too.

I volunteer to provide the method with tests if you like this approach.

Thanks for volunteering & please go for the PR! As you said, would be ideal to get this into 2.0 so we can mark SingleOrFallback obsolete and remove it entirely from a future version.

[atifaziz]

@fsateler I managed to get a slice of time to add this but would be interested in your review nonetheless. Documentation is also updated. I think best to open any issues as new or directly as a PR if it's just a minor correction.

[atifaziz]

FallbackIfEmpty is now also available in a 2.0 beta 2 package.

[fsateler]

@atifaziz I'll take a look when I get back home, I'm currently away. I
return next week.
On 9 Feb 2016 03:32, "Atif Aziz" notifications@github.com wrote:

FallbackIfEmpty is now also available in a 2.0 beta 2 package
https://www.nuget.org/packages/morelinq/2.0.0-beta02.

—
Reply to this email directly or view it on GitHub
#122 (comment).

[atifaziz]

@fsateler Thanks and I'll try and wait till then. 😉

[fsateler]

@atifaziz This seems to implement something different from what I had posted originally, and from what was requested in #12: a method taking a delegate. That is:

src.FallbackIfEmpty(() => new ExpensiveObject()).First();

The idea of FallbackIfEmpty would be to avoid a possibly expensive default object creation if the sequence is non-empty. So the current implementation does not quite replace the use of SingleOrFallback.

Would it make sense to have a FallbackIfEmpty<T>(this IEnumerable<T> source, Func<T> fallback) overload, or should this be a new method name?

[atifaziz]

I'm quite aware that the delegate overload is not there, but the functionality is not gone if you look at the following overload taking a sequence:

public static IEnumerable<T> FallbackIfEmpty<T>(
    this IEnumerable<T> source,
    IEnumerable<T> fallback)

The expensive object creation could then be wrapped into an iterator, with the call looking like src.FallbackIfEmpty(NewExpensiveObject()).First():

IEnumerable<ExpensiveObject> NewExpensiveObject() {
    yield return new ExpensiveObject();
} 

Now if only C# allowed anonymous iterator blocks, the call could read like src.FallbackIfEmpty(() => { yield return new ExpensiveObject(); }).First() then we wouldn't be discussing this. As we're not there, the fallback sequence covers that indirectly (and much more). The only downside is you have to write out a separate method but is that really a bad thing? My guess is that expensive object creation never looks as simple as a lambda expression so you'll end up writing a separate method anyhow because it may often require parameterization. Also, an expensive fallback object will be the more rare case so going out writing a whole separate iterator for that is acceptable. In terms of number of allocations, I believe the delegate and sequence overload are on par with each other, not to mention that the cost will be shadowed by the expensive object creation.

I was thinking that if an overload is needed for a lazy fallback value, wouldn't Lazy<T> be a much better fit as well as being self-evident through its signature?

[atifaziz]

@fsateler Also, wouldn't it be preferable to have more generalized ways of solving the problem than through overloads of each method, as in turn any function into a singleton sequence?

public static Seq class {
    public static IEnumerable<T> LazySingleton(Func<T> eval) { yield return eval(); }
}

So now you can write, albeit being slightly more verbose:

src.FallbackIfEmpty(Seq.LazySingleton(() => new ExpensiveObject()));

[fsateler]

@atifaziz Right, you can have the same functionality, although I'd argue that the intent of the code reads slightly different. I guess I can live with having my NewExpensiveObject method return an IEnumerable<T> instead of a T, and invoke it instead of passing the method.

I'm not sure I understand how would a Lazy<T> would fit in this scenario? Do you mean the overload would take a Lazy<T> instead of a Func<T>?

I think the same arguments you make with regards having a separate method LazySingleton would apply to the overloads taking fixed T parameters:

src.FallbackIfEmpty(Seq.FromValues(obj1, obj2));

So, in summary:

1. I agree the functionality is there, although not quite as obvious as an explicit delegate. I would prefer an overload with a delegate, but not oppose adding the method without the overload.
2. I don't think the static values overloads add much value (especially the single parameter one, there is already DefaultIfEmpty for that). It complicates the code with little extra benefit, as it can already be replaced with src.FallbackIfEmpty(new [] { obj1, obj2 }) with little extra cost.

Maybe it would be a good idea to add FromDelegates and FromValues methods to MoreEnumerable? For creating sequences based on Func<T> and T parameters respectively.

[atifaziz]

Do you mean the overload would take a Lazy<T> instead of a Func<T>?

Yes, like this:

public static IEnumerable<T> FallbackIfEmpty<T>(this IEnumerable<T> source, Lazy<T> fallback)

However, while that may have seemed like a good idea at first, I think Lazy<T> will give different semantics since the fallback value will only be created once and then shared among all iterations of the returned sequence. With Func<T>, the value would be created each time.

I think the same arguments you make with regards having a separate method LazySingleton would apply to the overloads taking fixed T parameters:

Not really. The purpose of those overloads is purely an optimization, to avoid the params array allocation for the simplest cases of up to 4 fallback values.

I agree the functionality is there, although not quite as obvious as an explicit delegate. I would prefer an overload with a delegate, but not oppose adding the method without the overload.

Great so let's leave it this way for 2.0 and we can add the delegate overload in a future minor release when it can be demonstrated that writing a separate iterator for the creation of an expensive object is more succinctly expressed as a lambda 80% to 90% of the time. Meanwhile, I agree that the trick is non-obvious so perhaps an example could help.

I don't think the static values overloads add much value (especially the single parameter one, there is already DefaultIfEmpty for that).

True, it is redundant but then is the right solution to rename FallbackIfEmpty to DefaultIfEmpty or remove the redundant overload? What would be odd is to have to switch methods out when you change the number of arguments. For example, imagine you're doing some ad-hoc analysis in LINQPad or even writing a code generator. It would be a shame to switch between DefaultIfEmpty and FallbackIfEmpty when changing between single and two or more fallback values. Also for code analysis, it helps to be able to find all references of FallbackIfEmpty irrespective of fallback value count. Don't you think?

It complicates the code with little extra benefit, as it can already be replaced with src.FallbackIfEmpty(new [] { obj1, obj2 }) with little extra cost.

Maybe it would be a good idea to add FromDelegates and FromValues methods to MoreEnumerable? For creating sequences based on Func<T> and T parameters respectively.

Could be a good idea. In any case it would be good to open those suggestions as separate issues.

[fsateler]

The purpose of those overloads is purely an optimization, to avoid the params array allocation for the simplest cases of up to 4 fallback values.

I see. But the same argument can be made for pretty much any other MoreLINQ method, no? I think the question is whether the optimization fit the semantics of the methods nicely or not. I think the meaning of this method is: "use a different sequence if the source is empty". While it is possible that many use cases do use a fixed, short sequence as fallback, this still feels like the meaning of the method gets confused a bit.

Great so let's leave it this way for 2.0 and we can add the delegate overload in a future minor release

Almost 👍

True, it is redundant but then is the right solution to rename FallbackIfEmpty to DefaultIfEmpty or remove the redundant overload?

I agree that it would be odd to have to change the method name as you change the number of arguments. Maybe a different option is to have MoreLINQ provide a few extra DefaultIfEmpty overloads with valued parameters, and have FallbackIfEmpty have exclusively an IEnumerable parameter. I think just removing the overloads with T parameters is a valid option as well.

Maybe it would be a good idea to add FromDelegates and FromValues methods to MoreEnumerable? For creating sequences based on Func and T parameters respectively.

Could be a good idea. In any case it would be good to open those suggestions as separate issues.

The intersection with this issue is that these methods would obsolete the version with T parameters. Filed as #150 as it makes sense to discuss anyway.

[atifaziz]

But the same argument can be made for pretty much any other MoreLINQ method, no?

Which other MoreLINQ extensions taking params are you thinking of that are not optimised in a similar fashion?

While it is possible that many use cases do use a fixed, short sequence as fallback, this still feels like the meaning of the method gets confused a bit.

Shall we let the beast loose and then see what sticks from future feedback? None of the overloads are logically doing anything different from what the description says.