-
Notifications
You must be signed in to change notification settings - Fork 419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FallbackIfEmpty overload with fallback factory #293
Comments
Indeed. In my local projects I have such an overload. I should probably clean it up a bit and post the PR... |
Cool! Please add. Eager to assist/review if necessary. |
There's a few and possibly more reusable and interesting ways to solve this. First of all, the desired effect can be achieved today using the var q =
Enumerable.Empty<int>()
.FallbackIfEmpty(EnumerableEx.Create(() => { return _(); IEnumerator<int> _()
{
yield return 1;
yield return 2;
yield return 3;
}})); The only downside is that it may be cumbersome to have all the syntactic ceremony if you're just returning one or two fallback values. But before you dismiss it on that basis, keep in mind that the above example is a toy one. In a more complex & realistic example such as the next one, the ceremony takes the back seat: var rates = Enumerable.Empty<(string Curr, double Rate)>();
var q =
rates.FallbackIfEmpty(EnumerableEx.Create(() => { return _(); IEnumerator<(string Curr, double Rate)> _()
{
const string url = "https://vincentarelbundock.github.io/Rdatasets/csv/datasets/euro.csv";
var csv = new WebClient().DownloadString(url);
var rows =
from row in Regex.Split(csv, @"\r?\n").Skip(1 /* headers */)
where row.Length > 0 // skip blank lines
select row.Split(',').Fold((curr, rate) => (Curr: curr, Rate: rate))
into row
select (row.Curr.Trim('"'), double.Parse(row.Rate));
return rows.GetEnumerator();
}})); Then again, you could just turn the whole rates download and parsing into a function: static IEnumerable<(string Curr, double Rate)> DownloadRates(string url) =>
from csv in new[] { new WebClient().DownloadString(url) }
from row in Regex.Split(csv, @"\r?\n").Skip(1)
where row.Length > 0
select row.Split(',').Fold((curr, rate) => (Curr: curr, Rate: rate))
into row
select (row.Curr.Trim('"'), double.Parse(row.Rate)); and consequently don't need rates.FallbackIfEmpty(DownloadRates("https://vincentarelbundock.github.io/Rdatasets/csv/datasets/euro.csv")); But let's suppose that we'd still rather supply value factories. I would then opt for public static IEnumerable<T> Values<T>(this IEnumerable<Lazy<T>> source) =>
from v in source select v.Value; Now that the return value is compatible with our var q =
Enumerable.Empty<int>()
.FallbackIfEmpty(new[] { new Lazy<int>(() => 123),
new Lazy<int>(() => 456) }.Values()); We could also introduce a (non-extension) public static IEnumerable<T> Values<T>(params Lazy<T>[] lazies) =>
lazies.Values(); This gets rid of the explicit array allocation at the call site: var q =
Enumerable.Empty<int>()
.FallbackIfEmpty(MoreEnumerable.Values(
new Lazy<int>(() => 123),
new Lazy<int>(() => 456))); And if we were to introduce another two helpers: public static IEnumerable<Lazy<T>> Lazy<T>(params Func<T>[] factories) =>
factories.Lazy();
public static IEnumerable<Lazy<T>> Lazy<T>(this IEnumerable<Func<T>> factories) =>
from f in factories select new Lazy<T>(f); then we get rid of explicit var q =
Enumerable.Empty<int>()
.FallbackIfEmpty(MoreEnumerable.Lazy(() => 123, () => 456).Values()); and now it doesn't read so bad. It's also very clear that those fallback values are served lazily. Bottom line, rather than just think of this in the context of one method like |
It's a joy to read your hardcore C# :) however few thoughts:
|
@abatishchev Semantically, you don't want eager evaluation of fallback values assuming they're costly so you turn to lazy evaluation. You can choose to do this technically with
Yes it does and it's getting better but it should still dwarf in comparison to the cost of computing the value. If you're worried about the cost of
If you don't want/need the cost of thread-safety synchronization then use
Wait, but that is
public static Func<T> ToFunc<T>(this Lazy<T> lazy) => () => lazy.Value; What am I missing? 🤔
We shouldn't boil this down to subjectivity or preferences of the caller where
And I think I've shown that public static IEnumerable<T> Values<T>(params Func<T>[] factories) =>
from f in factories select f(); and you're done: xs = xs.FallbackIfEmpty(Values(() => 1, () => 2, () => 3)); You're only downside will be one extra allocation of an enumerable and eventually an enumerator if the fallback values are needed. In practical terms, though, I would put those allocations and costs relative to (supposedly) those of fallback computations. Do you have a real example of how you came to your issue with |
@fsateler Can you share an example of how you've ended up needing that overload in your project? |
First of all, sorry for the silence. It's been a busy week and I haven't had time to fully think about @atifaziz concerns. I hope to do that soon though.
I have used it for example in classes that cache an expensive (that is, db) source: T GetById(int id) => LocalCache.Where(i => i.Id == id).FallbackIfEmpty(Repository.GetById).Single(); As you noted in your reply, you could do that using C# 7 and local functions: T GetById(int id) {
return LocalCache.Where(i => i.Id == id).FallbackIfEmpty(goToRepo).Single();
IEnumerable<T> goToRepo() {
yield return Repository.GetById(id);
}
} But I prefer to have that local function in a single place (the PS: I will fully respond to the longer post later, but I'll just quickly note that the |
But, is it always desired to evaluate once tame side effects? I don't think so. In particular, if the fallback function queries a mutable source, it may well be desirable to have the function be reevaluated on each enumeration. If side-effect taming or single-evaluation is desired, one can always use Memoize to "freeze" the sequence. In the case of public static Func<T> Memoize<T>(this Func<T> original) {
var lazy = new Lazy<T>(original);
return () => lazy.Value;
}
I agree with the general direction of your argument, but I disagree that the building block should be As I said back when |
I forgot to add, it is so common we already have a |
I'm sorry for silence too, last week before paternity leave as expected got to be a busy one.
Can't agree more. Not just due to small but overhead but first of semantically. Also original LINQ methods are all
It depends. In other words - we don't know. That's why I would leave that to the consumer to decide the desired behavior. So having two overloads would cover both scenarios. |
@fsateler Thanks for sharing that T GetById(int id) =>
LocalCache.Where(i => i.Id == id)
.FallbackIfEmpty(() => Repository.GetById(id)) // correction
.Single(); then it doesn't look like anything to do with sequences except that you start with one. You're really looking up a single value and failing that, you want to invoke some function to default one. We have T GetById(int id) => LocalCache.SingleOrDefault(i => i.Id == id)
?? Repository.GetById(id); If you trust your cache to have uniquely identifiable objects then you can also use If we had a type like T GetById(int id) => LocalCache.TryFind(i => i.Id == id) // if this returns None...
.OrElse(() => Repository.GetById(id)); // ...then the lambda runs But since that's not an option (pun, pun 😉), So now I'm still waiting for a good example to drive this. ⏳
Looks like we've already been here before and I should go back and refresh my memory so we don't keep repeating ourselves. I seem to have also mislead this discussion by mentioning side-effects. It was just pointed out as a side benefit of using |
@fsateler @abatishchev Guys, this is open source and volunteered time and work so no need to be sorry. 😆 |
Indeed, guilty as charged. In my defense, though, I imagined from the beginning that
The use cases in my mind always were about fetching a single value.
In this case yes, it is.
While not the primary motivation, it would introduce a semantic difference between
I don't understand this comment. Could you elaborate?
Is it? |
Regarding real-world scenarios, mine is quite simple: data source (Applier with U-SQL in Azure Data Lake) usually yields multiple rows but may also none. The layer above can't handle empty output since it relies on columns to present so it has to return a row with same columns as usually having nulls/default values. |
Regarding |
When #333 is merged this can probably be closed, as it would be as simple as |
Can it be as simple as |
@fsateler Or simpler yet: |
Is there a way to drop |
@abatishchev Technically speaking, there's always a way. If you can't bare the current design and wish to just pass the function as a parameter, then I suggest you create a tiny local wrapper to make it easy on your eyes: public static IEnumerable<T> FallbackIfEmpty<T>(
this IEnumerable<T> source, Func<T> fallbackFactory) =>
source.FallbackIfEmpty(MoreEnumerable.From(fallbackFactory)); |
@abatishchev And I should add that it's my bad (poor prioritisation or time management) for not getting back to @fsateler on the last points he raised and why this hasn't moved further along. |
Hi,
Currently FallBackIfEmpty has the following signature:
Does it mean if
T
is a method callprivate static T CreateDefault()
it would be called anyway, even ifsource
is not empty? I think yes, it would be.So I propose to add an overload:
which would call it if only
source
is indeed empty.Are you willing to accept a pr from me once agreed on the principles. Thanks!
The text was updated successfully, but these errors were encountered: