C# で for/foreach のかわりに Linq の All

Question

C# で for/foreach のかわりに（可能な限り）Linq の All を使用する者がいます。
使用者曰く「これといった副作用が無いから」とのこと。

// Using 'All'
var dressed = people.Awake().Brush().All(x => {
    x.clothes = suit;
    return true;
});

// Using 'foreach'
var dressing = people.Awake().Brush();
foreach (var x in dressing) {
    x.clothes = suit;
}

たしかにメソッドチェーンは見やすいとは思いますが、そもそも本来の All の意図を考えるとどうにも馴染めません。
はじめて当該コードを見たときは「？」でしたが、最後にあった「return true;」という一文を見てループしたいんだなと分かりました。

このような All の使用方法はわりと一般的なのですか？
速度について問われる場面ではなく、最終的な結果が同じならば、for/foreach のかわりに All を使用しても問題はないのでしょうか？

Answer 1

一般的ではないはずです。無駄なロジックが混ざって保守性を落としているので書き直すべきです。

どうしてもメソッドチェーンを使いたいならそのための拡張メソッドを書くのがいいですね。作ってあげたらどうですか？

Comments

[Zuishin]

people.Awake().Brush().Wear(suit).Execute() のようなメソッドチェーンを時々見ますが、これならいいと思います。条件を追加していって最後に確定するようなものですね。

[y-o-r-o]

やはり避けるべきですよね。
質問の意図を的確に汲み取って回答頂きありがとうございました。

Answer 2

All メソッドはシーケンスのすべての要素が条件を満たしているかどうかを判断ためのメソッドです。

本来の使用目的と異なる使い方すると可読性が落ちますので、このような書き方は避けるべきでしょう。

どうしても LINQ で書きたいのであれば、ForEach メソッドを使うべきかと思います。

var dressed = people.Awake().Brush().ToList().ForEach(x =>
{
    x.clothes = suit;
});

Comments

[hihijiji]

ForEachはList<T>のメソッドなので、前に.ToList()を付けたほうがいいかと

[nskydiving]

ご指摘ありがとうございます。
修正しました。

[y-o-r-o]

> 本来の使用目的と異なる使い方すると可読性が落ちますので、このような書き方は避けるべきでしょう。
やはりこれを遵守すべきですよね。ご回答ありがとうございました。

Answer 3

「これといった副作用が無いから」

peopleのclothesをsuitsにしているので、「副作用」しているように思います。こういうパターンではList.ForEachのようなものを使うのが普通に思います。

速度について問われる場面ではなく、最終的な結果が同じならば、for/foreach のかわりに All を使用しても問題はないのでしょうか？

問題のレベルによると思います。最終的な結果が同じ=問題はない、と考えることもできますし、保守性やリーダビリティまで問題に含めるなら問題ありだと思います。
(あくまで、foreachのかわりにLinqを使うことではなく、副作用させたいときに、Allを使うことが問題になり得るということです)

個人的には、Allでインスタンスを更新する開発者のコードは信用できないですね。他のところで、Allを使うことが適切であるところまで、疑ってかからないといけなくなるので。

Comments

[papinianus]

なぜ List<T>.ForEach は使うべきでないか
https://qiita.com/chocolamint/items/d29d699ce27bcf685154
コメントでAllで代用することの是非も触れられてます

[y-o-r-o]

ご回答ありがとうございます。
例文が問題ありましたね、すみません。
「副作用が無い」というのは「他の問題が無い」という意味だと捉えました（発言者もおそらくその意味）
なので、例文ではインスタンスのプロパティを更新していて誤解を招いてしまっていますが、単純にログを吐き出すだけでも構いません。聞きたかったことは「All は for の代替としていいのか？」です。

> 最終的な結果が同じ=問題はない、と考えることもできます
この考え方がマズイと思うので今回質問させて頂きました。

> 保守性やリーダビリティまで問題に含めるなら問題あり
やはりこれですね。

公式ドキュメントにも、
> Determines whether all elements of a sequence satisfy a condition.
とあるのは分かっておりましたが、はたしてそれが一般的にどこまで守られているのか判断がつかなかったため質問させて頂きました。
（現在のチームのエンジニア２名がどちらもAllを乱用。ちなみに私はエンジニアではありません・・・）

[papinianus]

> 単純にログを吐き出すだけ
一般にファイル出力は、副作用にあたります。IEnumerable<T>の要素の全てがlambdaの条件を見たすか否かの判定以外の処理をしている、というのが副作用の定義です。そのかたがやっていることこそ「副作用」であり、まさに「濫用」です。
> どこまで守られているか
赤信号みんなで渡れば怖くない理論だと思いますが、どんなにバッドノウハウが得られたとしても(得られませんでしたが)、MicrosoftDocにあわないコーディングはすべきではないです。

[y-o-r-o]

本来の意図にそぐわない処理そのものが「副作用」ということですね。
プログラミングの副作用と一般的な副作用のニュアンスが違うので今回使用すべきワードではありませんでしたね。すみませんでした。

[macof]

横からですいませんが１つ誤解がありそうなので失礼します。
一般的には副作用とは意図したかどうかには関係なく、式を評価した際に起きる状態変化を指すものです。
例えば代入演算子は代入を目的として使用しますが、
式としての評価結果は言語にもよりますが大抵は代入した値で、
変数内容の更新は副作用となります。

ファイル出力は出力先の状態を変化させるための処理なので原理的に副作用を持つというわけです。

[y-o-r-o]

> Example side effects include modifying a non-local variable, modifying a static local variable, modifying a mutable argument passed by reference, performing I/O or calling other side-effect functions.
Wikiにもありましたね。上記等の処理にて状態が変化することにより他に影響を与えてしまうことを副作用とするということですね。