patterncsharpMajor
Extension methods, is this too dirty?
Viewed 0 times
thistooextensionmethodsdirty
Problem
I love lambdas, and functional programming etc. But sometimes I wonder if I take it too far..
thoughts?
UPDATE
Some good points, some I agree with, some I don't, but a few details:
I understand that
The
The comments about using
To be honest I probably wouldn't use
public static T With(this T source, Action action)
{
action(source);
return source;
}
public static IEnumerable ForEach(this IEnumerable source, Action action)
{
return source.Select(x => x.With(action));
}thoughts?
UPDATE
Some good points, some I agree with, some I don't, but a few details:
I understand that
Linq may not be intended to handle side effects, especially since List.ForEach returns void, however I disagree that functional programming can't contain side effects, a monad is simply a chunk of code to execute for a given structure.The
ForEach and With extensions, are meant to be a syntactic sugar. Perhaps the name is poorly chosen, and it should be something like OnEnumerate, however I wonder if this could get confusing down the line, as its whole point is to invoke side effects at that particular point in the expression, and this could occur multiple times, perhaps I should stick with my With convention and do WithEach.The comments about using
Select internally are probably true, as I'm not really projecting, and yield return saves a method call. (premature optimization anyone?)public static IEnumerable WithEach(this IEnumerable source, Action action)
{
foreach(var x in source)
yield return x.With(action);
}To be honest I probably wouldn't use
WithEach often, unless its an edge case where I'm passing an IEnumerable that may or may not be enumerated. I can imagine it being effective in generating complex hierarchies. However With I use quite often, usually as a way to chain things together, i.e:this.SomeObservable = someObservable
.With(o => o.Subscribe(this.OnNext)
.With(this._disposables.Add));Solution
My advice: don't do this.
-
Building devices that encourage you to write expressions that are useful only for their side effects is poor functional style.
-
You are abusing "Select" when you use it as an implementation detail of "ForEach". "Select" does not have the semantics of "ForEach"! The purpose of "Select" is to compute a projection, not to cause a side effect.
-
You are building yourself a little time bomb. Because your implemenation of ForEach is to build a Select query, the "ForEach" executes lazily. When you say "blah.ForEach(whatever);" that doesn't do anything. It just builds up an object that represents doing the side effects in the future. Since you never use that object for anything, the side effects remain undone. If, by contrast, you do use that object, you run the risk of using it twice and causing the same side effects again.
That said, the idea of what you're doing is sound; the idea of representing a sequence of side-effecting operations in a lazily-evaluated object is how side effects are represented in Haskell. But if that's what you want to do, then actually build yourself a real monad that clearly and unambiguosly represents the semantics of "I'm building an object that represents a sequence of side effects in the future". Don't be all coy about it! Be explicit that that's what you're doing.
-
Building devices that encourage you to write expressions that are useful only for their side effects is poor functional style.
-
You are abusing "Select" when you use it as an implementation detail of "ForEach". "Select" does not have the semantics of "ForEach"! The purpose of "Select" is to compute a projection, not to cause a side effect.
-
You are building yourself a little time bomb. Because your implemenation of ForEach is to build a Select query, the "ForEach" executes lazily. When you say "blah.ForEach(whatever);" that doesn't do anything. It just builds up an object that represents doing the side effects in the future. Since you never use that object for anything, the side effects remain undone. If, by contrast, you do use that object, you run the risk of using it twice and causing the same side effects again.
That said, the idea of what you're doing is sound; the idea of representing a sequence of side-effecting operations in a lazily-evaluated object is how side effects are represented in Haskell. But if that's what you want to do, then actually build yourself a real monad that clearly and unambiguosly represents the semantics of "I'm building an object that represents a sequence of side effects in the future". Don't be all coy about it! Be explicit that that's what you're doing.
Context
StackExchange Code Review Q#3491, answer score: 25
Revisions (0)
No revisions yet.