この投稿は以下の記事を英語・JavaScriptから日本語（意訳）・PowerShell + 補足したものです。
5 Tips to Write Better Conditionals in JavaScript
JavaScriptやPowerShellなどの言語は少々言語ハックした記事が多いですが、
この投稿では原文通り言語ハックはせず、読みやすい書き方はこうだ、と思うものを書きました。
ソースコードはPowerShellですが、他の言語にも通ずるところはあると思います。
また、ところどころSonarQube（ソースコードの静的解析ツール）の規約の中から該当するルールを引っ張り出し、
なるべく、私見により過ぎないよう心がけてます。
不備などあれば、ご指摘ください。
この投稿で使用したソースコードはgistに置いてます。
環境
$PSVersionTable
Name Value
---- -----
PSVersion 5.1.17134.228
PSEdition Desktop
...
1. 包含演算子で論理演算子の数を減らす
下のコードは引数として渡ってきた果物を評価して、赤い果物であれば、'赤' と返す関数です。
function Test01_Bad($fruit) {
# リンゴ、イチゴ、サクランボであれば赤と判定する。
if ($fruit -eq 'リンゴ' -or $fruit -eq 'イチゴ' -or $fruit -eq 'サクランボ') {
return '赤'
}
}
Test01_Bad 'リンゴ' #=> 赤
一見すると、悪い書き方には見えないかもしれませんが、
例えば、リンゴ、イチゴ、サクランボに加えて、クランベリーやアセロラも入ってきた場合、
そのたびに-or演算子を追加しなければならないため、コードが冗長になってしまいます。
こういった場合、-in演算子を使った方がシンプルで読みやすいコードになります。
function Test01_Good($fruit) {
# リンゴ、イチゴ、サクランボであれば赤と判定する。
if ($fruit -in @('リンゴ', 'イチゴ', 'サクランボ')) {
return '赤'
}
}
-or演算子がなくなってスッキリしました。
SonarQubeでは一つの式で4つ以上の論理演算子を使うと警告が出るルールがあるので、
こういった場合は包含演算子が効きます。
https://rules.sonarsource.com/csharp/RSPEC-1067
補足：-containsを使うべきか、-inを使うべきか
PowerShellの包含演算子は-containsと-in（3.0~）があり、どちらも配列を左にとるか右にとるかの違いしかありません。
バージョンが特に問題なければ、比較する対象（あまり変化しない値、定数など）を右辺にとるような書き方が好ましいと思います。
if ($fruit -in @('リンゴ', 'イチゴ', 'サクランボ')) {
# もし `$fruit` がリンゴ、イチゴ、サクランボの中に含まれていれば
}
if (@('リンゴ', 'イチゴ', 'サクランボ') -contains $fruit) {
# もしリンゴ、イチゴ、サクランボが`$fruit`を含んでいれば
}
コメントに書いている通り、左辺と右辺を変えるだけで読み方が変わってしまいます。
この場合は、-in演算子で書いた方が素直に読むことができます。
詳細は、Wikipediaを参照してください。
ヨーダ記法
2. Avoid Else, Return Early の掟に従う
上の例で書いたコードに追加で条件を足していきます。
function Test02_Bad($fruit, $quantity) {
$redFruits = @('リンゴ', 'イチゴ', 'サクランボ', 'クランベリー')
# 条件1: $fruitに何かしらの文字列が入っていること。
if ($fruit) {
# 条件2: 赤い果物を抽出
if ($redFruits | Where-Object {$_ -in $fruit}) {
# 条件3: 量が一定数より多いこと
if ($quantity -gt 10) {
return '量が多い'
}
}
}
else {
throw '果物が指定されていません'
}
}
Test02_Bad 'リンゴ' 12 #=> 量が多い
上記のコードは全体的にネスト数が多くなっています。
SonarQubeでは4つ以上のネストだと可読性が著しく低下するとあるので、
3つ以上になってきた段階でなるべく減らすように書きたいところです。
https://rules.sonarsource.com/csharp/RSPEC-134
こういった場合は「Avoid Else, Return Early」という回避策？を適用できます。
function Test02_Good($fruit, $quantity) {
$redFruits = @('リンゴ', 'イチゴ', 'サクランボ', 'クランベリー')
# 条件1: エラーを早期にthrowします
if (!$fruit) {
throw '果物が指定されていません'
}
# 条件2: 赤い果実を抽出
if ($redFruits | Where-Object {$_ -in $fruit}) {
# 条件3: 量が一定数より多いこと
if ($quantity -gt 10) {
return '量が多い'
}
}
}
上記のコードは果物が指定されていない場合、早い段階でthrowすることにより、
一つのif~elseブロックが不要になったことで読みやすくなりました。
このように、Return Early は簡単にコードを改善できますが、
かといって、やりすぎてしまうとかえって読みづらいコードになってしまいます。
下のコードがその例です。
function Test02_OverdoReturnEarly($fruit, $quantity) {
$redFruits = @('リンゴ', 'イチゴ', 'サクランボ', 'クランベリー')
if (!$fruit) {
# 条件1: エラーを早期にthrowします
throw '果物が指定されていません'
}
if (!($redFruits | Where-Object {$_ -in $fruit})) {
# 条件2: 赤い果物でなければ処理を中断します
return
}
# 条件3: 量が一定数より多いこと
if ($quantity -gt 10) {
return '量が多い'
}
}
何もかもReturn Earlyしてしまうと、否定論理演算子が多くなり、
全体的な処理を一目で把握しづらくなります。
また、一つの関数内でreturnが多いことも、可読性を下げる危険性があります。
多くても3つ以下に抑えるのがいいと思います。
https://rules.sonarsource.com/java/RSPEC-1142
3. 引数の検査処理は関数の機能でなくす
例えば、以下のコードがあるとします。
function Test03_Bad($fruit, $quantity) {
if (!$fruit) {
throw '果物が指定されていません'
}
$quantity = if ($quantity) {
$quantity
}
else {
1
}
return "${quantity}個の${fruit}があります！"
}
Test03_Bad 'リンゴ' 1 #=> 1個のリンゴがあります！
この場合は引数にデフォルト値を指定した方がスッキリします。
function Test03_Good($fruit, $quantity = 1) {
if (!$fruit) {
throw '果物が指定されていません'
}
return "${quantity}個の${fruit}があります！"
}
また、$fruitが未指定の場合はthrowしておりますが、
[CmdletBinding()]属性を使用することで、これも省略することができます。
（もちろん、エラーメッセージは変わってしまいますが）
function Test03_Good_Advanced {
param(
[Parameter(Mandatory)]
[ValidateNotNull()]
$fruit,
$quantity = 1
)
return "${quantity}個の${fruit}があります！"
}
他にも[ValidateLength()]や[ValidatePattern()]、[ValidateScript()]などが利用できます。
詳しくは公式ドキュメントをご参照ください。
about_Functions_Advanced_Parameters
補足：三項演算子について
PowerShellでは三項演算子はサポートされていないため、上のコードのようにif~elseで書く必要があります。
別解のとして配列を使った書き方もありますが、やや言語ハックな書き方になるため、
コンソールで入力するコマンドやコードゴルフ以外ではあまり使用しないほうがいいと個人的には思います。
$quantity = @(1, $quantity)[[bool]$quantity]
4. switch文よりHashTableが単純でよい
以前、PowerShellのswitch文まとめを書いた際に思ったのが、PowerShellのswitch文の複雑さです。
他の言語と共通しない仕様が何点かあり、多くのケースでは問題ないですがやや使いづらい印象がありました。
冒頭で貼った元記事を読んだとき、「え、じゃPowerShellのswitch文って不要じゃない？！」って思ってしまいました。（これは言い過ぎ）
function Test04_Bad($color) {
# 色に該当する果物をswitch文で判定
switch ($color) {
'赤' {
return @('リンゴ', 'イチゴ')
}
'黄' {
return @('バナナ', 'パイナップル')
}
'紫' {
return @('ブドウ', 'ブルーベリー')
}
default {
return @()
}
}
}
Test04_Bad '赤' #=> @('リンゴ', 'イチゴ')
改めて書くと、長いですね。
function Test04_Good($color) {
# 色に該当する果物をHashTableで判定
$fruitColors = @{
"赤" = @('リンゴ', 'イチゴ')
"黄" = @('バナナ', 'パイナップル')
"紫" = @('ブドウ', 'ブルーベリー')
}
if ($color -in $fruitColors.Keys) {
return $fruitColors[$color]
}
return @()
}
HashTableを使うことで、スッキリ書けました。
探してみたら同様のルールがSonarQubeにもありました。
https://rules.sonarsource.com/csharp/RSPEC-1479
補足：スプラッティング（HashTableを使った実引数の指定方法）について
HashTableが出てきたので、ついでにスプラッティングについても触れておきます。
条件文とは関係ないですが、引数の多い関数をコールする際、
引数の指定順を間違えるとバグになることを避けるため、
PowerShellではスプラッティングという文法がサポートされています。
function Test04_Splatting($hoge, $fuga, $piyo, $hogera, $hogehoge, $fugafuga, $piyopiyo, $hogerahogera) {
$diff = Compare-Object -IncludeEqual @(
$hoge, $fuga, $piyo, $hogera, $hogehoge, $fugafuga, $piyopiyo, $hogerahogera
) @(
'hoge', 'fuga', 'piyo', 'hogera', 'hogehoge', 'fugafuga', 'piyopiyo', 'hogerahogera'
)
$diff.SideIndicator -notcontains '=>'
}
function Test04_SplattingDemo() {
$splatting = @{
hoge = 'hoge'; fuga = 'fuga'; piyo = 'piyo'; hogera = 'hogera'
hogehoge = 'hogehoge'; fugafuga = 'fugafuga'; piyopiyo = 'piyopiyo'; hogerahogera = 'hogerahogera'
}
Test04_Splatting @splatting
#=> True
}
状況次第なところもありますが、上のサンプル程度の状況だと
名前付き引数でコールした方がVisual Studio Codeの補完機能も使えるので、
2重指定や指定漏れが起こりにくく、可読性もそこまで悪くないため無理に使う必要はないかもしれません。
function Test04_SplattingPramName() {
Test04_Splatting `
-hoge 'hoge' -fuga 'fuga' -piyo 'piyo' -hogera 'hogera' `
-hogehoge 'hogehoge' -fugafuga 'fugafuga' -piyopiyo 'piyopiyo' -hogerahogera 'hogerahogera'
#=> True
}
5. ANY は -in 、ALLはForEach-Objectを上手く使う
これで最後の項目です。
LINQでいうEnumerable::Any、Enumerable::All、JavaのStream#anyMatch、Stream#allMatch、
JavaScriptでいうArray#some、Array#everyのPowerShellにおける書き方を紹介します。
5-1. ANY（一つ以上に一致）の場合
単純に-in演算子または、-contains演算子を使えば同じ処理となります。
念のため、悪い例、良い例と書いておきます。
function Test05_Bad_Any($fruits) {
$color = '赤'
$isAllRed = $false
# 条件: 一つ以上の果物が赤色であること
foreach ($fruit in $fruits) {
if ($isAllRed) {
break;
}
$isAllRed = $fruit.color -eq $color
}
return $isAllRed
}
$fruitsAnyRed = @(
@{name = 'リンゴ'; color = '赤'},
@{name = 'バナナ'; color = '黄'},
@{name = 'ブドウ'; color = '紫'}
)
$fruitsExceptRed = @(
@{name = 'バナナ'; color = '黄'},
@{name = 'ブドウ'; color = '紫'},
@{name = 'メロン'; color = '緑'}
)
Test05_Bad_Any $fruitsAnyRed #=> True
Test05_Bad_Any $fruitsExceptRed #=> False
-in演算子を使用。
ちなみに、$fruits.color で @('赤', '黄', '紫') と評価されます。
function Test05_Good_Any($fruits) {
$color = '赤'
# 条件: 一つ以上の果物が赤色であること（簡潔版）
$color -in $fruits.color
}
5-2. ALL（全てに一致）の場合
指定された色が全ての果物と一致することを評価するための関数です。
function Test05_Bad_Every($fruits) {
$color = '赤'
$isAllRed = $true
# 条件: すべての果物が赤色であること
foreach ($fruit in $fruits) {
if (!$isAllRed) {
break;
}
$isAllRed = $fruit.color -eq $color
}
return $isAllRed
}
$fruitsAllRed = @(
@{name = 'リンゴ'; color = '赤'},
@{name = 'イチゴ'; color = '赤'},
@{name = 'サクランボ'; color = '赤'}
)
$fruitsNotAllRed = @(
@{name = 'リンゴ'; color = '赤'},
@{name = 'イチゴ'; color = '赤'},
@{name = 'メロン'; color = '緑'}
)
Test05_Bad_Every $fruitsAllRed #=> True
Test05_Bad_Every $fruitsNotAllRed #=> False
PowerShellには -allmatch演算子や AllMatch-Objectコマンドレットみたいな機能は標準では
サポートされていないですが、ForEach-Object（エイリアスは%）で代用できます。
function Test05_Good_Every_Pipeline($fruits) {
$color = '赤'
# 条件: すべての果物が赤色であること（簡潔版）
$fruits.color | % {$ret = $true} {$ret = $ret -and $_ -eq $color} {$ret}
}
ただ、この書き方は Begin, Process, Endブロックの処理の流れを知っておく必要があるので、
現場によればforeach文で書いた方がいいかもしれません。
（そこまで知っていない人でも何となく理解できる気がしなくもないですが）
また、別解としてLINQを使った書き方もできます。
function Test05_Good_Every_Linq($fruits) {
$color = '赤'
# 条件: すべての果物が赤色であること（LINQ版）
[System.Linq.Enumerable]::All(
$fruits.color,
[System.Func[object, bool]] {
param($x) $x -eq $color
}
)
}
ただ、PowerShellだとC#でいう拡張メソッドが使用できないため、ちょっとした処理の割には既述が多くなるようです。
また、私みたいに普段JavaJavaしている人にとってははっきりと読みやすいコードとは言えないかもです。
現場がC#やVBエンジニアばかりであれば、問題ないと思いますが。
（むしろ、PowerShell使うような現場は.Netメインが当たり前ですかね、どうなんでしょう。）