在包装器中包装琐碎的功能是一种好习惯吗？

Question

我正在进行代码审查，我注意到开发人员已经这样做了：

UserSession.LocationId = CheckInteger(elementValue);

使用这个包装器

    private int CheckInteger(string elementValue)
    {
        int outNumber;
        int.TryParse(elementValue, out outNumber);
        return outNumber;
    }

我看不出这会给聚会带来什么。我应该退缩，还是让熟睡的狗躺下？我认为没有任何特定的公司政策涵盖这一点。

Answer 1

他们是睡狗还是狼？此代码有效地丢弃非整数值，并为失败的情况返回 0。

这可能合适，也可能不合适，但名称肯定会产生误导 - 根本不执行任何检查。错误被简单地丢弃。这几乎相当于隐藏异常。

当未检测到无效值并且在意想不到的地方设置了 0 时，这可能伤害您。

这样的方法应该至少重命名为GetIntegerOrDefault，这样它就会在出错时返回一个默认值。

您还应该确保 0 是 LocationId 的有效值或至少是预期的默认值。否则你可能会遇到一些很难诊断的错误。

我在一个证券交易所应用程序中遇到了错误，该错误会发送 -100% 的投资组合绩效比率，因为下面 10 个级别的某个人决定在无效汇率值上返回 0，而 5 年后没有人记得。

Answer 2

鉴于在这种情况下您可以写 int.TryParse(elementValue, out UserSession.LocationId) 我会说这不是一个好主意。

现在，如果您需要为其添加日志记录或以一致的方式处理特殊情况/错误，那么请务必这样做。