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 个新函数：一个是这个新的抽象，另一个只是一个助手，它调用新函数然后执行第 5 行。这些函数中的一个可能比另一个具有更长的预期使用寿命。 .

于 2010-11-14T15:32:25.963 回答
2

对我来说，试金石是：如果我需要在一个地方更改此代码序列（例如添加一行，更改顺序），我是否需要在其他位置进行相同的更改？

如果答案是肯定的，那么它是一个逻辑的“原子”实体，应该重构。如果答案是否定的，那么它是一系列操作碰巧在不止一种情况下工作 - 如果重构，将来可能会给您带来更多麻烦。

于 2010-12-22T06:20:58.460 回答
1

我最近在考虑这个问题，我完全理解你的意思。在我看来，这是一种实现抽象，因此如果您可以避免更改接口，它会更容易接受。例如，在 C++ 中，我可能会将函数提取到 cpp 中，而无需触及标题。这在一定程度上减少了函数抽象对类用户的格式良好和有意义的要求，因为在他们真正需要它之前（添加到实现时）它对他们是不可见的。

于 2010-11-14T16:07:47.580 回答
1

对我来说，最重要的词是“门槛”。另一个词可能是“气味”。

事情总是处于平衡状态。听起来（在这种情况下）平衡的中心是凝聚力（酷）；而且由于您只有少量重复项，因此管理起来并不难。

如果你发生了一些重大的“事件”并且你去了“1000”个重复，那么平衡就会改变，你可能会接近。

对我来说，一些可管理的重复并不是重构的信号（还）；但我会留意它。

于 2010-11-18T03:26:17.293 回答
0

传承是你的朋友！

不要重复代码。即使单行代码很长或很困难，也要将其重构为具有独特名称的单独方法。把它想象成一个会在一年内阅读你的代码的人。如果您将此函数命名为“blabla”，那么下一个人会在不阅读它的代码的情况下知道该函数的作用吗？如果没有，您需要更改名称。经过一周这样的思考，你会习惯它，你的代码的可读性会提高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 回答