コードのifやelseを減らしてきれいにしたい

Question

タイトルの通り，elseやifが多いコードをきれいにしたいです．
コードを見てください．

process.stdin.resume();　
process.stdin.setEncoding('utf8');

let R = "D"
let W = "D"
let winner = "B";

let lines = []; 
let reader = require('readline').createInterface({
    input: process.stdin,
    output: process.stdout
});
reader.on('line', (line) => {
    lines.push(line);
});
reader.on('close', () => {
    for(let i = 0; i < lines.length; i++){
        if(lines[i] == "RD"){
            if(R == "D"){
                winner = "A"
            }else{
                R = "U"
            }
        }else if(lines[i] == "RU"){
            if(R == "U"){
                winner = "A"
            }else{
                R = "U"
            }
        }else if(lines[i] == "WD"){
            if(W == "D"){
                winner = "A"
            }else{
                W = "D"
            }   
        }else if(lines[i] == "WU"){
            if(W == "U"){
                winner = "A"
            }else{
                W = "U"
            }
        }
     }
    console.log(winner)
});

これは，旗上げゲームです．

標準入力で赤白の旗(R，W)を上げるか下げるか(U，D)の命令を適当な数入力します．

最初は赤白両方の旗を下げている状態で初めて，命令に間違いがあった場合(旗が下がっているのに下げる命令をする等)はAがwinnerになり，間違った命令をしなかった場合はBがwinnerになります．

コードのelseやifがごちゃごちゃしていて気になるのですが，どのように改善すればよいかわからず質問させていただきました．

解答の程よろしくお願いします．

Comments

[stdio]

個人的には、貴方のコードのごちゃごちゃ感は味があって良いと思いますよ。

[hentaiman]

それが気になるので改善したいようですが。

[miyabi_pudding]

回答しといてなんですが、十中八九paizaとかですよね。

[TTaro]

ある企業のインターンの選考で，このようなコードを書けという問題が出て，自分で回答したものがごちゃごちゃしていたのでうまい人はどのように書くのだろうかと思って質問しました．
問題の内容を具体的に書いてしまいました．
もう少しぼかして書くべきでした．
すみませんでした．

[miyabi_pudding]

> もう少しぼかして書くべきでした．
すみませんでした．

それは…
そうですね、選考問題なら、尚更問題文出しちゃまずいですね。
選考員とかもエンジニアなら、teratailはけっこう見られてます。

今回は、自身のコードで動くものなので、いいのかな？と思いますが、
いらぬ誤解を生んで、質問者さんの不利益になる可能性が十分あります。

Answer 1

ふむ、paizaっぽいのでアウトっぽく見えますが、
自力で動作するようなコードが書けているようなのでセーフとみなして回答します。

ブラウザのデベロッパーツールやNode.jsのREPLで
下記のコードを試してみましょう。

> ("RD")[0]
"R"

> var [a, b] = "RD"
"RD"

> a
"R"

> b
"D"

このようにJavaScriptの文字列(String)は
日本語のようなマルチバイト文字さえ絡まなければ、
配列のように1文字ずつ取り出す事が可能です。

---

次に、変数は文字から探すと骨が折れます。
これがif文だらけになる主原因です。

ここは汎用オブジェクトを作って
キー名で管理するようにしましょう。

> state = {R: "D", W: "D"}

> state.R
"D"

> state["R"]
"D"

> color = "R"
"R"

> state[color]
"D"

> state[color] = "U"
"U"

> state
{R: "U", W: "D"}

これで"R"や"W"の文字から値を読み書き出来るようになります。

---

最後にfor文で作られるiはどうせ不要なので、
既に何かしらの配列が存在して1個ずつアイテムを抜き出す構造にする場合
for...ofを使いましょう。

ただし、これは単純にコードが10倍簡素になり読みやすくなる程度の効果しかありません。

---

上記3点を全て含めたコードがこちら
制御の為にif文を2個追加しましたが、
コア部分の重複は抑えられ、まぁまぁ短くスッキリした見た目になったかと思います。

const state = {
  R: "D",
  W: "D"
};
let winner = "B";

reader.on('close', () => {
  for (const line of lines) {
    const [color, order] = line;
    // 無効となる命令を取り除く
    if (color != "R" && color != "W") continue;
    if (order != "D" && orser != "U") continue;
    if (state[color] == order) {
      winner = "A";
      break; // 負け確定後は検証する意味がないのでループを抜ける
    }
    state[color] = order;
  }
  console.log(winner);
});

書きづらい時は事前準備で工夫する。
これを意識するようにしてみてください。

Comments

[miyabi_pudding]

> JavaScriptの文字列(String)は
日本語のようなマルチバイト文字さえ絡まなければ、
配列のように1文字ずつ取り出す事が可能です。

（勉強になりました）

[miyabi-sun]

因みにJSはUTF-8でやっているので、
マルチバイトも意外と頑張れる仕様なんだけど
絵文字のような5バイト文字はES5までの仕様だと使えないのかな？
ES2015以降に追加された機能だとイケるみたいな裏事情があります

```
> ("????????")[0]
"�" // <- 化けてしまった……

> [a, b] = "????????"
"????????"
> a
"????"
> b
"????"
```

Answer 2

class HandStruct {
  constructor(color, action = 'D') {
    this.color = color;
    this.action = action
  }
  exchange(actionFlg) {
    this.action = actionFlg ? 'U' : 'D';
  }
}

const Player = {
  R: new HandStruct('R'),
  W: new HandStruct('W')
};

// 中略

reader.on('close', () => {

let winner = "";

const programs = lines.map( a => {
  const aarr = a.split("");
  return new HandStruct(aarr[0], aarr[1]);
});

programs.some(p => {
  if ( Player[p.color].action == p.action ) {
    winner = 'A';
    return true;
  } else {
    winner = 'B';
    Player[p.color].exchange(p.action == 'U');
  }
});

console.log(winner);

});

Comments

[miyabi_pudding]

整理するコツは、同じような処理は二度と書かない、という事を意識することです。
メンテナンス時も、修正箇所が減り、メンテナンス性を向上できます。