选择基本描述符

Question

这段代码非常大，看起来很混乱。我试图对其进行重构，使其更具可读性，使代码看起来更少.(滚动下面)。不过，我不知道这是否可以再加考虑。

 TableTypeDescriptor baseDecriptor = base.CustomTypeDescriptor as TableTypeDescriptor;

        if (this.IsObjectBeingValidated)
            return baseDecriptor; 

        if (base.CustomTypeDescriptor != null)
        {
            if (baseDecriptor.OldTableDescriptor != null && this.IsNewInCorrection && !this.IsReadOnly)
            {
                return base.CustomTypeDescriptor;
            }
            else if (this.Locked)
            {
                return base.CustomTypeDescriptor;
            }
            else if (this.ImportedDataMD5 != null && !this.IsReadOnly)
            {
                return base.CustomTypeDescriptor;
            }
            else
            {
                if (baseDecriptor.OldTableDescriptor != null)
                {
                    return baseDecriptor.OldTableDescriptor;
                }
                else
                    return base.CustomTypeDescriptor;
            }
        }

        return base.CustomTypeDescriptor;

然后，我将其重新分解如下。是否有可能更多地重构它？

TableTypeDescriptor baseDescriptor = base.CustomTypeDescriptor as TableTypeDescriptor;

                if (base.CustomTypeDescriptor != null || !this.IsObjectBeingValidated)
                {
                    if ( (!this.IsReadOnly && (this.IsNewInCorrection || this.ImportedDataMD5 != null))  ||
                         this.Locked                                
                       )
                        baseDescriptor = base.CustomTypeDescriptor;
                    else
                    {
                        if (baseDescriptor.OldTableDescriptor != null)
                            baseDescriptor = baseDecriptor.OldTableDescriptor;
                        else
                            baseDescriptor = base.CustomTypeDescriptor;
                    }
                }

                return baseDescriptor;

Answer 1

您也应该对单个{}语句使用大括号if。这使得您的代码不太容易出错。

如果您正在重构某些代码，那么您应该确保有良好的测试，以验证重构代码的行为是否与旧代码相同。

通过将所有这些this.xx()属性的检查提取到一个单独的方法，如

private static bool ShouldReturnCurrentDescriptor()
{
    return (this.IsObjectBeingValidated) ||
           (this.Locked) ||
           (!this.IsReadOnly && (this.IsNewInCorrection || this.ImportedDataMD5 != null));
}  

以前的代码可以重构为

TableTypeDescriptor baseDecriptor = base.CustomTypeDescriptor as TableTypeDescriptor;

if (ShouldReturnCurrentDescriptor() ||
    (baseDecriptor.OldTableDescriptor == null))
{
    return baseDecriptor;
}
return baseDecriptor.OldTableDescriptor;  

Answer 2

这里有一些东西可以使代码更短(注意:它使代码更短，但并不总是更易读)：

1.在赋值时，可以使用? :进行if-else构造。例如：

if(i > 5)
{
    x = i;
}
else
{
    x = 0;
}

可重写为：

x = (i > 5) ? i : 0;

2.可以提前返回以减少嵌套。例如：

public void someMethod(int i){
    if(i > 5)
    {
        int x = i * 10;
        x += 5;
        someOtherMethod(x);
    }
}

可重写为：

public void someMethod(int i){
    if(i <= 5) return;

    someOtherMethod((i * 10) + 5);
}

以下只是一些关于如何缩短代码的例子。

现在让我们回到您的代码：

：

我可以把它缩短到：

TableTypeDescriptor baseDescriptor = base.CustomTypeDescriptor as TableTypeDescriptor;

if (base.CustomTypeDescriptor == null || this.IsObjectBeingValidated) return baseDescriptor;

baseDescriptor = ((!IsReadOnly && (IsNewInCorrection || ImportedDataMD5 != null)) || Locked)
    ? base.CustomTypeDescriptor;
    : ((baseDescriptor.OldTableDescriptor != null)
        ? baseDescriptor.OldTableDescriptor
        : base.CustomerTypeDescriptor);
return baseDescriptor;

这是一个短得多，如你可以看到，但它肯定不是更可读的！相反，您需要在短代码和可读性之间找到一个中间。就我个人而言，如果我有你的代码，我会把它修改成这样：

TableTypeDescriptor baseDecriptor = base.CustomTypeDescriptor as TableTypeDescriptor;
if(baseDescriptor == null || base.CustomerTypeDescriptor) return null;

if(IsObjectBeingValidated) return baseDescriptor;

if((baseDescriptor.OldTableDescriptor != null && IsNewInCorrection && IsReadOnly)
    || (ImportDataMD5 != null && IsReadOnly)
    || Locked)
{
    return base.CustomTypeDescriptor;
}

return baseDescriptor.OldTableDescriptor != null
    ? baseDescriptor.OldTableDescriptor
    : base.CustomTypeDescriptor;

也许您还可以将您的大型if分离成一个单独的方法。因此，与其：

if((baseDescriptor.OldTableDescriptor != null && IsNewInCorrection && IsReadOnly)
    || (ImportDataMD5 != null && IsReadOnly)
    || Locked)
{
    return base.CustomTypeDescriptor;
}

你可以做到：

if(checkSomething())
{
    return base.CustomTypeDescriptor;
}

...

// TODO: obviously change this name. I don't know what your application is about, so can't really think of a good name for it
private void checkSomething(){
    return (baseDescriptor.OldTableDescriptor != null && IsNewInCorrection && IsReadOnly)
        || (ImportDataMD5 != null && IsReadOnly)
        || Locked;
}

我个人并不认为它像这样可读性太强，但是如果没有完整的代码，我就无法对其进行足够的重构。

总之，编写更易读的代码并不总是容易的，它需要实践和知识。此外，还有一些很好的重构工具可以让事情变得更简单。我目前在我的ReSharper WPF项目中使用C#，并且可以推荐它。

Answer 3

有人在上面提到了这一点，但我觉得他们在回答时并没有给予足够的重视。

您的代码不符合C#标准，很难阅读

TableTypeDescriptor baseDescriptor = base.CustomTypeDescriptor as TableTypeDescriptor；if (base.CustomTypeDescriptor != null if！!this.IsObjectBeingValidated) { if ( (!this.IsReadOnly && (this.IsNewInCorrection欧元/ this.ImportedDataMD5 != null) \x/ this.Locked )\x/base.CustomTypeDescriptor) baseDescriptor = base.CustomTypeDescriptor；否则{if (baseDescriptor.OldTableDescriptor =null) baseDescriptor = baseDecriptor.OldTableDescriptor；否则baseDescriptor = base.CustomTypeDescriptor；}返回baseDescriptor；

始终使用括号，您可以在没有括号的情况下编写一行if语句，但是当它使用括号时，它不会产生可读的代码。

每当在of语句中有if/ look语句时，就应该考虑可能使用else if语句来减少嵌套。

我使用外部if语句作为短路，通过返回baseDescriptor退出该方法。

一开始，我查看了我留下的内容，我说只有一个实例需要返回baseDescriptor.oldTableDescriptor，这就让我了解了以下内容：

TableTypeDescriptor baseDescriptor = base.CustomTypeDescriptor as TableTypeDescriptor;

if (base.CustomTypeDescriptor == null || this.IsObjectBeingValidated)
{
    return baseDescriptor;
}

if (baseDescriptor.OldTableDescriptor != null)
{
    baseDescriptor = baseDecriptor.OldTableDescriptor;
}
else
{
    baseDescriptor = base.CustomTypeDescriptor;
}
return baseDescriptor

但是，我想到了您令人困惑的if语句，并且认为您很可能有很好的理由这么做，所以我不得不让它单独使用，但是我确实将整个结构更改为if/else if/ with语句，并在结构中返回，如下所示：

TableTypeDescriptor baseDescriptor = base.CustomTypeDescriptor as TableTypeDescriptor;

if (base.CustomTypeDescriptor == null || this.IsObjectBeingValidated)
{
    return baseDescriptor;
}

if ((!this.IsReadOnly && (this.IsNewInCorrection || this.ImportedDataMD5 != null)) || this.Locked)
{
    return base.CustomTypeDescriptor;
}
else if (baseDescriptor.OldTableDescriptor != null)
{
    return baseDecriptor.OldTableDescriptor;
}
else
{
    return base.CustomTypeDescriptor;
}

我认为没有理由在最初的if语句中设置条件2行。