不要从许多方法中分配一个字段

Question

我的 class:

中有这样一个字段

public class Shape
{
    private Point2D m_location;

    public void Move()
    {
       m_location = ...
    }

    public void Rotate()
    {
       m_location = ...
    }

    public void Flip()
    {
       m_location = ...
    }
}

我在 NDepend 中收到一条警告：

Don't assign a field from many methods

https://www.ndepend.com/default-rules/Q_Don't_assign_a_field_from_many_methods.html

看来我可以通过在 class 中创建另一个方法轻松解决这个问题，例如：

private void SetLocation(Point2D point)
{
    m_location = location;
}

并在所有设置 m_location 的方法中调用它。现在 m_location 仅在一种方法中分配！

这是解决这个问题的有效方法吗？我是否只是通过在这些方法中调用 SetLocation() 方法来隐藏之前检测到的代码味道 NDepend？

Answer 1

Is this a valid way to solve this problem?

没有。正如您所怀疑的，这是一种代码味道。 NDepend 抱怨的是可变引用；你有代码在哪里：

var s = new SomeObject(someInitialization);
var r = s.SomeResult();
// you now have no idea what s contains or if it is even usable any more.

解决方案是使 SomeObject 不可变和 return 新引用而不是更改内部结构：

public SomeObject Something()
{
    return new SomeObject(SomethingDifferentDependingOn(this.something));
}

现在你有：

而不是你的第一个例子

var s = new SomeObject(someInitialization);
var r = s.Something().Result;
// s is guaranteed to be unchanged.

是的，有时您需要可变引用。在那些情况下；记录它们并解释为什么它们必须是可变的。然后你可以在 case-by-case 的基础上 override NDepend rules 来防止它显示警告。如果您有代码气味，请警告人们。不要试图隐藏它。

你编辑后的例子有很大的不同，但总的原则仍然成立。如果您只有几个内部字段在方法调用中全部更改，您仍然可以 return 不可变引用，例如：

public Shape Move()
{
    return new Shape(m_location ...);
}

如果你有很多不会全部改变的内部字段，或者你需要做一些事情，比如共享私有字段，你就不能轻易地拥有不可变的引用，但你仍然可以通过使用访问器来避免警告：

public Location
{
    get { return m_location; }
    private set { m_location = value; }
}

然后在您的内部方法中专门使用 Shape.Location。

不要从许多方法中分配一个字段

Don't assign a field from many methods

c#

refactoring

ndepend