7

コードの複製とリファクタリングに関する質問があります。一般的すぎないことを願っています。非常に低いレベルではない一連の関数呼び出しであるかなり小さなコード (〜 5 行) があるとします。このコードはいくつかの場所で繰り返されているため、ここでメソッドを抽出することをお勧めします。ただし、この特定の例では、この新しい関数は凝集度が低いという問題があります (これは、特に、関数の適切な名前を見つけるのに苦労することによって明らかになります)。その理由はおそらく、この繰り返されるコードがより大きなアルゴリズムの一部に過ぎず、適切な名前のステップに分割するのが難しいためです。

そのようなシナリオであなたは何を提案しますか?

編集：

より多くの人が役立つ可能性があるように、質問を一般的なレベルに保ちたかったのですが、コードサンプルでバックアップするのが最善であることは明らかです. この例はこれまでで最高のものではないかもしれません (かなり多くの点でにおいがします) が、うまくいくことを願っています。

class SocketAction {

    private static class AlwaysCreateSessionLoginHandler extends LoginHandler {
        @Override
        protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException {
            Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID());
            socketAction.registerSession();
            socketAction._sess.runApplication();
        }
    }

    private static class AutoConnectAnyDeviceLoginHandler extends LoginHandler {
        @Override
        protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException {
            if (Server.isUserRegistered(socketAction._sess.getUserLogin())) {
                Log.logSysInfo("Session autoconnect - acquiring list of action threads...");
                String[] sa = Server.getSessionList(socketAction._sess.getUserID());
                Log.logSysInfo("Session autoconnect - list of action threads acquired.");
                for (int i = 0; i < sa.length; i += 7) {
                    socketAction.abandonCommThreads();
                    Server.attachSocketToSession(sa[i + 1], socketAction._commSendThread.getSock());
                    return;
                }
            }
            Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID());
            socketAction.registerSession();
            socketAction._sess.runApplication();
        }
    }

    private static class OnlyNewSessionLoginHandler extends LoginHandler {
        @Override
        protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException {
            socketAction.killOldSessionsForUser();
            Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID());
            socketAction.registerSession();
            socketAction._sess.runApplication();
        }
    }
}
4

5 に答える 5

8

質問は一般的すぎて実際に言うことはできませんが、演習として:

それを抽象化するとします。結果の 5 行関数を変更したい理由として考えられるのは何かを考えてみてください。すべてのユーザーに適用される変更を行いたいと思いますか?それとも、一部の呼び出し元が変更を必要とする理由があるたびに、古い関数とは少し異なる新しい関数を作成しなければならなくなりますか?

すべてのユーザーに対して変更したい場合、それは実行可能な抽象化です。今は悪い名前を付けてください。後でもっと良い名前を考えるかもしれません。

将来コードが進化するにつれて、この関数を多くの同様のバージョンに分割することになる場合、それはおそらく実行可能な抽象化ではありません。関数を作成することもできますが、それは問題の正式なモデルの一部というよりも、コードを節約する「ヘルパー関数」です。これはあまり満足のいくものではありません。この量のコードの繰り返しは、どこかに実行可能な抽象化が必要であることを示唆しているため、少し心配です。

たぶん、5 行のうち 4 行はよりまとまりがあるので、抽象化することができます。次に、2 つの新しい関数を作成できます。1 つはこの新しい抽象化であり、もう 1 つは新しい関数を呼び出して 5 行目を実行する単なるヘルパーです。これらの関数の 1 つは、他の関数よりも長い耐用年数を持つ可能性があります。 .

于 2010-11-14T15:32:25.963 に答える
2

私にとってリトマス試験紙は次のとおりです。この一連のコードを 1 か所で変更する必要がある場合 (行の追加、順序の変更など)、他の場所でも同じ変更を行う必要があるでしょうか?

答えが「はい」の場合、それは論理的な「アトミック」エンティティであり、リファクタリングする必要があります。答えが「いいえ」の場合、それはたまたま複数の状況で機能する一連の操作であり、リファクタリングすると、将来さらに問題が発生する可能性があります。

于 2010-12-22T06:20:58.460 に答える
1

私は最近これについて考えていました、そして私はあなたが何を得ているのかを正確に理解しています. これは何よりも実装の抽象化であり、インターフェースの変更を避けることができれば、より好ましいと思います。たとえば、C++ では、ヘッダーに触れずに関数を cpp に抽出する場合があります。これにより、(実装に追加するときに) 本当に必要になるまで関数の抽象化が見えないため、クラスのユーザーにとって関数の抽象化が整形式で意味のあるものであるという要件がいくらか軽減されます。

于 2010-11-14T16:07:47.580 に答える
1

私にとって有効な言葉は「しきい値」です。これの別の言葉はおそらく「匂い」でしょう。

物事は常にバランスがとれています。（この場合）バランスの中心が凝集性（クール）にあるように聞こえます。少数の重複しかないため、管理は難しくありません。

いくつかの大きな「イベント」が発生し、重複が「1000」に達した場合、バランスがずれており、接近している可能性があります。

私にとって、いくつかの管理可能な重複は、(まだ) リファクタリングのシグナルではありません。しかし、私はそれを監視します。

于 2010-11-18T03:26:17.293 に答える
0

継承はあなたの友達です！

コードを複製しないでください。1 行のコードが非常に長くて難しい場合でも、それをリファクタリングして、固有の名前を持つ別のメソッドにします。1 年であなたのコードを読む人のように考えてください。この関数に「blabla」という名前を付けた場合、次の人はコードを読まなくてもこの関数が何をするかを知ることができますか? そうでない場合は、名前を変更する必要があります。そのように 1 週​​間考えた後、あなたはそれに慣れ、あなたのコードは12%より読みやすくなります! ;)

class SocketAction {

    private static abstract class CreateSessionLoginHandler extends LoginHandler {
        @Override
        protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException {
            Server.checkAllowedDeviceCount(socketAction._sess.getDeviceID());
            socketAction.registerSession();
            socketAction._sess.runApplication();
        }
    }

    private static class AlwaysCreateSessionLoginHandler extends CreateSessionLoginHandler;

    private static class AutoConnectAnyDeviceLoginHandler extends CreateSessionLoginHandler {
        @Override
        protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException {
            if (Server.isUserRegistered(socketAction._sess.getUserLogin())) {
                Log.logSysInfo("Session autoconnect - acquiring list of action threads...");
                String[] sa = Server.getSessionList(socketAction._sess.getUserID());
                Log.logSysInfo("Session autoconnect - list of action threads acquired.");
                for (int i = 0; i < sa.length; i += 7) {
                    socketAction.abandonCommThreads();
                    Server.attachSocketToSession(sa[i + 1], socketAction._commSendThread.getSock());
                    return;
                }
            }
            super.onLoginCorrect(socketAction);
        }
    }

    private static class OnlyNewSessionLoginHandler extends CreateSessionLoginHandler {
        @Override
        protected void onLoginCorrect(SocketAction socketAction) throws IllegalAccessException, IOException {
            socketAction.killOldSessionsForUser();
            super.onLoginCorrect(socketAction);
        }
    }
}
于 2010-11-24T22:14:58.830 に答える