プログラムの別の書き方

Question

以下のコードは、

flgにチェックが入ったら、PHASE10に
yoteibiに値が入ったら、statusをPHASE50に
kakuteibiに値が入ったら、statusをPHASE60に
jisshibiに値が入ったら、statusをPHASE70に

更新しようとしているプログラムです。

それ以外に、flgにチェックが入っても、
もしstatusがPHASE20,30,40の場合は、PHASE10にしない制限があります。

さらに、yoteibiにおいても、yoteibiの値を消して、
statusがPHASE50だった場合で、
かつ、flgにチェックがあれば、PHASE10にステータスを遷移させています。

kakuteibiやjisshibiもyoteibiと同じような感じになっています。

このソース一見わかりにくいので、シンプルに書き換える方法あれば
どんな感じにできるでしょうか。

function setStatus(newIntro) {

        if (data.flg && (new Set<String>{'PHASE20', 'PHASE30', 'PHASE40'}).contains(data.status)) {
            // do nothing
        } else {
            data.status = 'PHASE10';
        }

        if (data.yoteibi != null
                && data.status != 'PHASE60'
                && data.status != 'PHASE70'
                ) {
            data.status = 'PHASE50';
        } else if (data.yoteibi == null
                && data.status != 'PHASE50'
                && data.flg) {
            data.status = 'PHASE10';
        }

        if (data.kakuteibi != null && data.status != 'PHASE70') {
            data.status = 'PHASE60';
        } else if (data.kakuteibi == null
                && data.status != 'PHASE60') {
            if (data.yoteibi != null) {
                data.status = 'PHASE50';
            } else if (data.flg) {
                data.status = 'PHASE10';
            }
        }

        if (data.jisshibi != null) {
            data.status = 'PHASE70';
        } else if (data.jisshibi == null
                && data.status != 'PHASE70') {
            if (data.kakuteibi != null) {
                data.status = 'PHASE60';
            } else if (data.yoteibi != null) {
                data.status = 'PHASE50';
            } else if (data.flg) {
                data.status = 'PHASE10';
            }
        }
        

}

Comments

[KSwordOfHaste]

JavaではなくJavascriptなので質問のタグを変更しておいてください。

[m.ts10806]

PHPは関係なさそうですね。質問のタグをはずしておいてください。

[kei344]

new Set<String>{}という記法はブラウザで動くJavaScriptでは無いと思うのですが、どういった環境でどの言語を使われているのでしょうか。

Answer 1

意見は分かれるかと思いますが、個人的な意見を書きます。

このソースで既に動いているのであれば処理は変えない方が良いと思います。
ただし説明頂いた内容をコメントとして書いておくと良いです。

Answer 2

ちょっと要件を詰め切れてないので参考までにですが...。
とりあえずフィルタで上書いていくというイメージでしょうでしょうか?
「タイプコード自身がやるべきことを知っててif文が少ない」
というのがOOPらしさともいえます。どちみちリテラルが頻出するのはよろしくないのでenum使ってみる。
※ちなみに基本的なことですがJavaの文字列比較は==使わない。

//仕込み
public enum Phase {
	NONE(0,"NONE",false),
	FLG(10,"PHASE10",false),
	HOGE_1(20,"PHASE20",true),
	HOGE_2(30,"PHASE30",true),
	HOGE_3(40,"PHASE40",true),
	YOTEIBI(50,"PHASE50",false),
	KAKUTEIBI(60,"PHASE60",false),
	JISSHIBI(70,"PHASE70",false);
	private Phase(final int code,final String name,final boolean fix){
		this.code = code;
		this.name = name;
		this.fix = fix;
	}
	private final int code;
	private final String name;
	private final boolean fix;//この値が入ってる場合変更が無い

	//文字列をenumに変換
	static Phase getPhase(final String nm){
		for(Phase p: values()){
			if(p.name.equals(nm))
				return p;
		}
		return NONE;
	}
	Phase checkFilter(final String key,final Phase checkPhase){
		//チェック対象がNULLの場合そのまま
		//PHASE20,30,40の場合なにもしない
		if(key == null || this.fix)
			return this;
		//チェック対象に値がある場合、紐づくコード値が大きい方で置き換え
		return checkPhase.code > this.code ? checkPhase : this;

	}
	//自身の結果取得
	String getName(){
		return this.name;
	}
}

//使い方/////////////////////////////////////
public static void main(String[] args) {
	Data data = new Data();
	data.status = "PHASE50";
	//data.jisshibi = "xx";
	data.flg = "xx";
	data.jisshibi = "xx";

	Phase nowPhase = Phase.getPhase(data.status);
	//ここの順番はどうでもいい
	nowPhase = nowPhase.checkFilter(data.flg, Phase.FLG);
	nowPhase = nowPhase.checkFilter(data.yoteibi, Phase.YOTEIBI);
	nowPhase = nowPhase.checkFilter(data.kakuteibi, Phase.KAKUTEIBI);
	nowPhase = nowPhase.checkFilter(data.jisshibi, Phase.JISSHIBI);
	data.status = nowPhase.getName();
	System.out.println("status="+data.status);//PHASE70

}
static class Data{
	String flg;
	String yoteibi;
	String kakuteibi;
	String jisshibi;
	String status;
}

Comments

[退会済みユーザー]

ご回答ありがとうございます。
Enumですか。Enumはホント単純なものしかあつかったことがなく、勉強になります。

NONE(0,"NONE",false),
や
static Phase getPhase(final String nm){
などのようなことができたりできるんですね。。

[KSwordOfHaste]

蛇足ながら、kurokobaさんの回答はJavascriptではなく、Javaです。
考え方は参考にするとしても言語が違いますので、Javascriptで同じようには書けません。
to:keetonさん

[kurokoba]

すみません、回答時点でJavaタグが付いていたので(そこから修正してません)...。
JavaScriptのOOP(オブジェクト指向プログラミング)には賛否両論あってOOPが良いからこれに倣え、とはいえませんがOOPのプログラミングの思想自体は言語に依存しないものです。
「if文(Switch)をポルモーフィズムへ書き換える」という考え方はOOPに基本的なものですし、M.ファウラー「リファクタリング」でも取り上げられています。
またステータスの定数自体にメソッドを持たせて
if(XXX) { doXXX}…
みたいなものを分岐なしで書き換えるというのはGoFのデザインパターンでStateパターンの一種ともいえます。
※まあJavaのenumやScalaのcaseクラスがStateパターン書きやすいというのはありますが...。

Answer 3

質問内容とソースから察するに、大きいPHASEの条件ほど優先されると見受けられるので、
以下の処理で問題ないのではないでしょうか。

function setStatus(newIntro) {
        String resultStatus = data.status;//一応変数に格納
        if (data.flg && (new Set<String>{'PHASE20', 'PHASE30', 'PHASE40'}).contains(data.status) ==false) {
            resultStatus = 'PHASE10';//20～40以外でflgオンなら10
        }

        if (data.yoteibi != null) {
            resultStatus = 'PHASE50';//50未満上書き
        }

        if (data.kakuteibi != null) {
            resultStatus = 'PHASE60';//60未満上書き
        }

        if (data.jisshibi != null) {
            resultStatus = 'PHASE70';//70未満上書き
        }
        data.status = resultStatus;//結果を設定
}

Comments

[退会済みユーザー]

ご回答ありがとうございます。

最初私も、上記と似たコードを書いていたのですが、

縦おば、jisshibiが空になったときに、PHASE70のままではなく、PHASE60などに戻すことができると理想で、

そうしたところ、ifが複雑な感じになってしまいました。

[monagano]

jisshibiが空でPHASE60になるタイミングでは、kakuteibiは空なのでしょうか？
kakuteibiが空でなければ、上記処理を通過するとおのずとPHASE60になると思います。

[退会済みユーザー]

あ、なるほど！すみません、昨日の私の返信が間違えていました。
頂いたコードでシンプルかつ、網羅できそうですね。
試してみます。ありがとうございます。