質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

90.35%

  • Java

    16756questions

    Javaは、1995年にサン・マイクロシステムズが開発したプログラミング言語です。表記法はC言語に似ていますが、既存のプログラミング言語の短所を踏まえていちから設計されており、最初からオブジェクト指向性を備えてデザインされています。セキュリティ面が強力であることや、ネットワーク環境での利用に向いていることが特徴です。Javaで作られたソフトウェアは基本的にいかなるプラットフォームでも作動します。

  • デザインパターン

    83questions

    デザインパターンは、ソフトウェアのデザインでよく起きる問題に対して、解決策をノウハウとして蓄積し再利用出来るようにした設計パターンを指します。

  • リファクタリング

    17questions

    リファクタリングとはコードの本体を再構築するための手法であり、外見を変更せずに内部構造を変更/改善させることを指します。

State/Strategeによるタイプコードの置き換えについて(Java言語で学ぶリファクタリング入門)

解決済

回答 3

投稿

  • 評価
  • クリップ 2
  • VIEW 1,872

oshigotoDacho

score 51

Java言語で学ぶリファクタリング入門 (結城 浩 著)の第9章<State/Strategeによるタイプコードの置き換え>について質問させていただきます。

以下にリファクタリング前後のコードと動作用のMainクラスを記載します。

//リファクタリング前
public class Logger {
    public static final int STATE_STOPPED = 0;
    public static final int STATE_LOGGING = 1;
    private int _state;
    public Logger() {
        _state = STATE_STOPPED;
    }
    public void start() {
        switch (_state) {
        case STATE_STOPPED:
            System.out.println("** START LOGGING **");
            _state = STATE_LOGGING;
            break;
        case STATE_LOGGING:
            /* Do nothing */
            break;
        default:
            System.out.println("Invalid state: " + _state);
        }
    }
    public void stop() {
        switch (_state) {
        case STATE_STOPPED:
            /* Do nothing */
            break;
        case STATE_LOGGING:
            System.out.println("** STOP LOGGING **");
            _state = STATE_STOPPED;
            break;
        default:
            System.out.println("Invalid state: " + _state);
        }
    }
    public void log(String info) {
        switch (_state) {
        case STATE_STOPPED:
            System.out.println("Ignoring: " + info);
            break;
        case STATE_LOGGING:
            System.out.println("Logging: " + info);
            break;
        default:
            System.out.println("Invalid state: " + _state);
        }
    }
}
//リファクタリング後
public class Logger {
    private enum State {
        STOPPED {
            @Override public void start() {
                System.out.println("** START LOGGING **");
            }
            @Override public void stop() {
                /* Do nothing */
            }
            @Override public void log(String info) {
                System.out.println("Ignoring: " + info);
            }
        },

        LOGGING {
            @Override public void start() {
                /* Do nothing */
            }
            @Override public void stop() {
                System.out.println("** STOP LOGGING **");
            }
            @Override public void log(String info) {
                System.out.println("Logging: " + info);
            }
        };

        public abstract void start();
        public abstract void stop();
        public abstract void log(String info);
    }

    private State _state;
    public Logger() {
        setState(State.STOPPED);
    }
    public void setState(State state) {
        _state = state;
    }
    public void start() {
        _state.start();
        setState(State.LOGGING);
    }
    public void stop() {
        _state.stop();
        setState(State.STOPPED);
    }
    public void log(String info) {
        _state.log(info);
    }
}
//動作用Mainクラスはリファクタリング前後で変化せず

public class Main {
    public static void main(String[] args) {
        Logger logger = new Logger();
        logger.log("information #1");

        logger.start();
        logger.log("information #2");

        logger.start();
        logger.log("information #3");

        logger.stop();
        logger.log("information #4");

        logger.stop();
        logger.log("information #5");
    }
}


疑問点は以下になります。

余分なインスタンスが作成されていい理由は?
リファクタリング前のコードは、メソッドが呼び出された時に、Stateが該当する状態だったら何もしないとなっています。
(STATE_LOGGINGならばstart()を呼び出した時、STATE_STOPPEDならばstop()を呼び出した時)
しかし、リファクタリング後のコードでは、setState(State.~)を実行して新たにインスタンスを作成してしまいます。
実行結果はリファクタリング前と変化はありませんが、余分なインスタンスが作成されてしまっていいのでしょうか?

自己流でダメな点は?
以下は自分でリファクタリングを行ったコードです。
こちらもメソッドが呼び出される度に新たなインスタンスを作成していますが、①が問題ないのでしたら、このコードも問題ないのでしょうか?

//自己流リファクタリング後
public class Logger {
    private State _state;
    public Logger() {
        State state = new StateStopped();
        setState(state);
    }
    public void setState(State state) {
        this._state = state;
    }
    public void start() {
        _state.start();
        setState(new StateLogging());
    }
    public void stop() {
        _state.stop();
        setState(new StateStopped());
    }
    public void log(String info) {
        _state.log(info);
    }
}

public abstract class State {
    public abstract void start();
    public abstract void stop();
    public abstract void log(String info);
}

public class StateLogging extends State {
    @Override public void start() {
        System.out.println();
    }
    @Override public void stop() {
        System.out.println("** STOP LOGGING **");
    }
    @Override public void log(String info) {
        System.out.println("Logging: " + info);
    }
}

public class StateStopped extends State {
    @Override public void start() {
        System.out.println("** START LOGGING **");
    }
    @Override public void stop() {
        System.out.println();
    }
    @Override public void log(String info) {
        System.out.println("Ignoring: " + info);
    }
}

以上になります。どなたかご教示お願い致します。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

checkベストアンサー

+2

  1. 余分なインスタンスは作成されません。
     列挙型enumと言うのは静的不変インスタンスの集合です。列挙された要素はstatic finalなインスタンスのような特徴を持ち、enumのクラスがロードされた段階で列挙された要素(この場合LOGGING,STOPPED)のインスタンスが作成されて変数として保持されます。setStateで渡しているインスタンスはこのStateクラスですでに作成されているインスタンスへの参照を代入していることにほかなりません。setStateの段階でインスタンスを作成しているわけではありません。
     それよりもこのリファクタリングで目的としていることは、想定外の状態を代入しないことです。元のコードでは、状態としてintの値を利用しています。しかし、状態を表すのは0と1のみで、他は「異常な状態」です。しかし、int値としては当然それ以外の値も入り得るため、安全性に欠けます。そこで状態に列挙型を利用すると、列挙型で挙げられたインスタンス以外を指定することが不可能になるため(指定するとコンパイルエラーになる)、安全性が高まります。

  2. StateLogging,StateStopped以外のStateを作成され、想定外の動きをさせられる恐れがあります。
    前述のとおりenumを使った場合新たなインスタンスを生成しないのに対し、こちらは新たなインスタンスをメソッドの起動のたびに生成するというデメリットが有ります。仮にこの2つのクラスのインスタンスを1つ生成し、どこかのクラスのstatic finalフィールドにおいておき、それを使えばそのような問題はなくなります。
    しかし一方で、引数として受け取っているのが抽象クラスStateであることから、ユーザーが自身でこれを継承し、独自の実装をしてこのメソッドに渡すことが可能になってしまいます。セキュリティの観念からこれは望ましくないことは明白です。Stateクラスをパッケージプライベートにして、2つの状態クラスをfinalに指定するという方法が考えられますが、それらをすべて実現できるenumの利便性には劣ります。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

+2

①余分なインスタンスが作成されていい理由は?
そもそもenumはシングルトンになるため、インスタンスの増減はしません。setStateは定数の代入に近いものですね。

②自己流でダメな点は? 
enumが導入される前はこのようなコードで対応していたと推測されます。
ただし、現状のコードでは状態のシングルトンな性質を捉えていない点や拡張が自由な点が問題かなと思います。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

+1

こんにちは。

先ずご質問に回答させていただく前にですが、
start() と stop() ですが、継承先でどちらか一方が要らなくなっています(機能退化してます)。

ご提示いただいた例だけを考えますと、startもstopも、Loggerクラスの機能としたほうがふさわしく、
一方でStateがやるべきことはstartかstopされたときに、何らかのアクションを起こすものとお見受けします。

ですから、Stateから start と stop メソッドを取り去り、代わりに initメソッドにしてしまったほうが良いかと思われます。

private enum State {
        STOPPED {
            @Override public void init() {
                System.out.println("** START LOGGING **");
            }
            @Override public void log(String info) {
                System.out.println("Ignoring: " + info);
            }
        },

        LOGGING {
            @Override public void init() {
                System.out.println("** STOP LOGGING **");
            }
            @Override public void log(String info) {
                System.out.println("Logging: " + info);
            }
        };

        public abstract void init();
        public abstract void log(String info);

そのうえで、Loggerクラスのstartおよびstopメソッドは次のように変更します。

public void start() {
        setState(State.LOGGING);
        _state.init();
    }
    public void stop() {
        setState(State.STOPPED);
        _state.init();
    }

stateのイニシャライズが必須であれば、できればsetStateメソッドは外部公開しないように設計するほうが望ましいでしょう。

これが適うならば、内部の _state を入れ替えるだけですので、setStateメソッド自体がいらなくなります。

さて、ご質問いただいた ① に関しましてですが、都度インスタンスが作られることを気にされるのであれば、Singletonで設計すると良いでしょう。

ただし、Singletonにしてもインスタンスが増えることには変わりありません。
メモリに常駐しますので、その分メモリは消費します。

ですが、メモリを消費してStateパターンを採用したところ、
ご提示の例で言えば、ログが「停止状態」と「実行状態」の実装を別々に分けて記述できるようになります。

例えば「受注が未だ」「受注確度高い」「受注済」「在庫あり」「発注前」「納品前」・・・
これらの状態のもと、一つの機能(取引状況お知らせメールとか)が使われなければならない、と想定します。

きっとStateパターンを利用しない場合はif文やswitch文を使って、あらゆるところで条件分岐が使われることになるのは想像できるでしょうか。
ソースの見通しが悪くなり、保守効率が下がります。改修の際にバグが発生する確率も上がります。

Stateパターンによりメモリが消費されることと、保守性能を下げることとを比較し、どちらに重点を置くか。
感情的にならず、冷静に、公平に判断され、最適な方法である根拠が示せるのであれば、どちらの方法をとっても良いと考えます。

startおよびstopメソッドの「機能の退化」を改善する前提であれば、②の方法も十分立派だと思います。
都度インスタンスすることが気になるようであれば、StateはSingletonにしてしまいましょう。
私もこの場合においてStateパターンを採用するのであればSingletonにします。

public class StateLogging extends State {

    private SateLogging(){}
    private static StateLogging _me;
    public static StateLogging instance(){ 
      if(_me == null) _me = new StateLogging();
      return _me;
    }

    @Override public void init() {
        System.out.println("** STOP LOGGING **");
    }
    @Override public void log(String info) {
        System.out.println("Logging: " + info);
    }
}

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2016/01/27 12:07

    うーん…言わんとしていることはわかるのですが…
    これだと「開始状態で開始しようとする」とか、「停止状態で停止しようとする」とかの、現実的に起こりそうな操作が全く考慮されていないのが気になります。
    さらにMainの書き換えも必要になり、当初の目的からずれているように思います。

    キャンセル

  • 2016/01/27 14:31

    swordone さん

    こんにちは。ご指摘ありがとうございます。

    機能退化を改善することに関してのご指摘でしょうか?

    Loggerクラスには sart と stop メソッドは実装されています。
    MainロジックはLoggerクラスだけが見える状態ですので、書き換えは必要ないです。

    「開始状態で開始しようとする」と「停止状態で停止しようとする」の点につきましては、その処理に目的があり、startメソッドもstopメソッドも、継承先で有意に実装されるのであれば、残すべきです。

    ご質問内容を拝見したところ、
    ①start、stopメソッドが何も実装されない場合がある
    ②インスタンスを都度作成する
    という2か所あると考えました。

    ①を前提とされる場合、やはりこれは実装されないメソッドですので、「そのうち使うかもしれない」という無駄な機能を残すべきではなく、機能の退化を避けるべきと考えます。
    繰り返しますが、有意な実装が成されるならば、startもstopも残すべきです。

    ご質問内容に外れるところは断りつつ、できるだけくみ取って沿って回答差し上げたつつもりですが・・・「当初の目的とずれている」と感じられたのはどのあたりでしょうか?

    キャンセル

  • 2016/01/27 15:08

    「当初の目的とずれている」というご意見の論拠に、気が付きました!

    swordoneさんは、リファクタリングの目的を
    「想定外の状態を代入しないこと」に置いてますね。

    私のほうは、リファクタリングの目的は、Sate/Strategyのパターンを採用されているため、
    「区分による条件分岐があちこちに散見されるような効率の悪いコードにしない」
    という前提のもと、

    ①インスタンスを都度つくるのは大丈夫か
    ②EnumではなくAbstractのクラスでも良いか

    を質問されているものと考えてました。

    加えて、①の質問の中に、「start と stop の実装が要らない場合もあるのは良いのか」という質問もあるのかと考えてもいます。

    どちらがご質問者の意図なのでしょう・・・

    キャンセル

  • 2016/01/29 17:21

    ご回答ありがとうございます。
    >①の質問の中に、「start と stop の実装が要らない場合もあるのは良いのか」
    これは意図していませんでした。しかし、仰るとおり無駄な機能は残すべきではないですね。自分でコードを組むときは心がけるようにします。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 90.35%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

同じタグがついた質問を見る

  • Java

    16756questions

    Javaは、1995年にサン・マイクロシステムズが開発したプログラミング言語です。表記法はC言語に似ていますが、既存のプログラミング言語の短所を踏まえていちから設計されており、最初からオブジェクト指向性を備えてデザインされています。セキュリティ面が強力であることや、ネットワーク環境での利用に向いていることが特徴です。Javaで作られたソフトウェアは基本的にいかなるプラットフォームでも作動します。

  • デザインパターン

    83questions

    デザインパターンは、ソフトウェアのデザインでよく起きる問題に対して、解決策をノウハウとして蓄積し再利用出来るようにした設計パターンを指します。

  • リファクタリング

    17questions

    リファクタリングとはコードの本体を再構築するための手法であり、外見を変更せずに内部構造を変更/改善させることを指します。

  • トップ
  • Javaに関する質問
  • State/Strategeによるタイプコードの置き換えについて(Java言語で学ぶリファクタリング入門)