TimeZone计算方法

Question

我觉得这里面有种暗号味。有人能帮我重构/优化它吗？

protected TimeZone resolveTimeZone() {
    IUserPreferences preferences = userProfileService.getCurrentUserPreferences();
    if (preferences != null) {
        String timeZone = preferences.getPreferredTimeZone();
        if (timeZone != null && !"".equals(timeZone))
            return TimeZone.getTimeZone(timeZone);
        else return defaultTimeZone();
    }
    else return defaultTimeZone();

}

public TimeZone defaultTimeZone() {
    Locale defaultLocale = new Locale(appConfiguration.getDefaultLocale());
    Calendar calendar = Calendar.getInstance(defaultLocale);
    return calendar.getTimeZone();
}

Answer 1

乍一看，一些立即改进的想法是可能的：

• 避免重复代码：defaultTimeZone()出现两次。通过更改分支逻辑可以很容易地考虑到这一点:继续检查需求(preferences != null、首选项值null等)，直到找到可用的值并返回为止。若要处理所有其他(失败)执行路径，请在方法的末尾使用单个返回。
• 使用.isEmpty()方法检查字符串是否为空。这比!"".equals(value)好，意思是“这个值与""相同”。使用isEmpty是很自然的。
• 名为timeZone的变量表示它是TimeZone的一个实例。这是误导，我会把它重命名为其他东西来澄清。

适用上述规定：

protected TimeZone resolveTimeZone() {
    IUserPreferences preferences = userProfileService.getCurrentUserPreferences();
    if (preferences != null) {
        String timeZoneText = preferences.getPreferredTimeZone();
        if (timeZoneText != null && !timeZoneText.isEmpty()) {
            return TimeZone.getTimeZone(timeZoneText);
        }
    }
    return defaultTimeZone();
}

仔细看，还有一些我不喜欢的东西：

• defaultTimeZone方法从发布的代码中的其他getter调用中略显出来：getCurrentUserPreferences、getPreferredTimeZone、getDefaultLocale、.也许最好与这些方法保持一致，并将方法重命名为getDefaultTimeZone。
• 打电话给Calendar.getInstance是很昂贵的。如果默认时区在应用程序的生存期内不会改变，那么最好在启动时初始化它一次，然后再使用它。
• resolveTimeZone这个名字有点模糊。“决心”是什么意思，真的？从实现的角度来看，它并不像可能的那样清晰。

我建议将这种方法分割成这样：

protected TimeZone getTimeZone() {
    TimeZone preferredTimeZone = getPreferredTimeZone();
    if (preferredTimeZone != null) {
        return preferredTimeZone;
    }
    return defaultTimeZone();
}

private TimeZone getPreferredTimeZone() {
    IUserPreferences preferences = userProfileService.getCurrentUserPreferences();
    if (preferences != null) {
        String timeZoneText = preferences.getPreferredTimeZone();
        if (timeZoneText != null && !timeZoneText.isEmpty()) {
            return TimeZone.getTimeZone(timeZoneText);
        }
    }
    return null;
}

在这里，方法的名称和实现更自然：

• getTimeZone：查看实现，非常清楚发生了什么:如果可能的话使用首选时区，否则使用默认的时区。又好又简单。
• getPreferredTimeZone：从方法名称来看，很自然，该方法试图从用户配置中计算出一个时区，否则返回null。

唉，与原来的情况相比，这确实有额外的if条件的缺点。然而，在实践中，额外调用的费用应该可以忽略不计。这可能是品味的问题。我认为设计中清晰性的价值超过了额外if的成本(当然，除非性能绝对重要)。

Answer 2

我会像这样重写resolveTimeZone：

protected TimeZone resolveTimeZone() {
    IUserPreferences preferences = userProfileService.getCurrentUserPreferences();
    if (preferences != null) {
        String timeZone = preferences.getPreferredTimeZone();
        if (timeZone != null && !timeZone.equals("")) {
            return TimeZone.getTimeZone(timeZone);     
        }  
    }
    return defaultTimeZone();
}

注意，我将else分支从最内部的if中移除。这是不必要的。如果时区检查失败，函数将在最后一个return语句处返回。

我也代替了尤达的条件。我个人不喜欢尤达的条件，我相信它们不会增加任何价值，尤其是在你的情况下。timeZone.equals("")被读取为“时区等于空字符串”，而"".equals(timeZone)被读取为“空字符串等于timeZone”。从逻辑上讲，平等是可以交换的，但是第二种版本对大多数人来说并不是自然的。

在最后一个支撑之前，我还删除了空行。它有点缺乏对细节的关注，并且打破了围绕它的相当一致的代码。我想这是我发现的第一种代码气味。

我对defaultTimeZone()没什么大问题。也许这个名称更适合于一个字段，而不是一个执行操作的函数？

除了以上所有的内容之外，您的代码非常干净，易于解析。