将字符串中的字符(A)移5位

Question

这段代码中没有语法错误，因此可以在性能、可维护性和可用性方面进行改进。

 class Program
    {
        static void Main(string[] args)
        {
            int shift = 5;
            string output = "";
            Console.Write("Input: ");
            string input = Console.ReadLine();
            if (input != null)
            {
                for (int i = 0; i < input.Length; i++)
                {
                    if (input[i] < 65 || input[i] > 90)
                    {
                        throw new Exception("Only A-Z supported.");
                    }
                    int shifted = input[i] + shift;
                    if (shifted > 90)
                    {
                        shifted = 65 + shifted - 91;
                    }
                    output = output + (char)shifted;
                }
            }
            Console.WriteLine("Output: " + output);
        }
    }

Answer 1

建议1

您的shift变量可以是常量(除非另有规定，您应该尝试并拥有所有常量)。但是，它只使用一次，所以我认为通过将值移到for循环并完全删除变量，不会失去可读性：

for (int i = 0; i < input.Length; i++)
{
    ...

    // Hardcode the value of 5, rather than referencing shift
    int shifted = input[i] + 5;
    
    ...
}

我想可以提出这样的论点:将shift值存储在变量中可以让您在以后的某个时间点轻松地修改密码，但在这种情况下，最好将所有逻辑从static void Main中提取出来，并以任何方式将shift函数作为参数！

建议2

这种逻辑：

shifted = 65 + shifted - 91;

可以简化为

shifted -= 26;

建议3

如果要在循环中连接字符串，则应该使用StringBuilder。(要导入using System.Text;，需要在.cs文件的顶部包括StringBuilder)

StringBuilder output = new StringBuilder();

...

for (int i = 0; i < input.Length; i++)
{
    ...

    // We now append to the StringBuilder, rather than the string
    // Deleted: output = output + (char)shifted;
    output.Append((char) shifted);
}

...

此外，还有一个小小的建议，我会移动输出的声明，以便在调用Console.ReadLine()之后，但在for循环之前。这是尽可能晚地声明变量，并且只提高(在我看来)可读性(但我知道有些人更喜欢提前声明所有内容！)

建议4

您应该反转if语句的逻辑，该语句检查空输入，以减少程序的总体嵌套：

if (input == null) {
    Console.WriteLine("You didn't provide an input!");
    return; // This is important otherwise it will still try and perform the rotation!
}

...
// Now you can continue, knowing that input is not null

建议5

可以用前程循环替换for循环：

foreach (char c in input)
{
    // Now you don't have to use `input[i]`, you can just use `c`
}

对于foreach循环，您会丢失循环的索引(i)，但是在解决方案中没有使用它，所以它适合您的用例。

Answer 2

if (input[i] < 65 || input[i] > 90)

将更具可读性

                    if (input[i] < 'A' || input[i] > 'Z')

类似地，

shifted = 65 + shifted - 91;

可能是

                    shifted -= (1 + 'Z' - 'A');

或后数学

                    shifted -= 26;

我会发现26远比65 - 91更容易辨认。因为我知道(Latin1)字母表中有26个字母。如果您认为您的代码的大多数读者可能不知道这一点，那么最好使用字母‘字符常数来使您实际上正在做的事情更清楚。

修复后验误差的1可能需要注释。但如果你用字母表的大小，那你就不需要这个了。添加常量可能会有所帮助：

const char AlphabetSize = 1 + 'Z' - 'A';

那么你不需要解释26是字母表的大小。常量的名字为你记录。现在，不太重要的是，文档1避免了fencepost错误，因为人们可以自己处理逻辑。显然，我们正在计算字母表size...what，我们需要这样做吗？