TDD风格的字符串计算器Kata码

Question

我已经完成了以下TDD风格的kata，如果有人能检查我的代码和测试，我会很感激的。

串计算器

• 使用方法String创建一个简单的int Add(string numbers)计算器。该方法可以接受0、1或2个数字，并返回它们的和(对于空字符串，它将返回0)。例如"“或"1”或"1,2“。
• 从一个空字符串的最简单的测试用例开始，然后移动到1和两个数字。
• 记住尽可能简单地解决问题，这样你就可以强迫自己写一些你没有想过的测试。
• 记住，每次通过测试后都要重构。
• 允许Add方法处理数量未知的数字。允许Add方法处理数字之间的新行(而不是逗号)。以下输入是确定的：“1\n 2，3”(将等于6)。下面的输入是不确定的："1，\n“(不需要证明它-只是澄清)。
• 支持不同的分隔符来更改分隔符，字符串的开头将包含如下所示的单独行："//分隔符\n数字…“。例如，"//;\n1;2“应该返回默认分隔符为';‘的3。第一行是可选的。仍然应该支持所有现有的方案。
• 用负号调用Add将抛出一个异常“不允许”--并抛出所传递的负数。如果有多个否定句，请在异常消息中全部显示。

public class StringCalculator
    {
        public int AddNumbers(string args)
        {
            if (string.IsNullOrEmpty(args))
            {
                return 0;
            }

            var delimeters = new List<char>()
                                    {
                                        '\n',','
                                    };

            if (args[0] == '/')
            {
                var customDelimeter = args[2];
                delimeters.Add(customDelimeter);
                args = args.Remove(0,
                                   3);

            }

            var numbers = args.ToCharArray().Where(x => !delimeters.Contains(x)).ToList();

            if (numbers.Any(x => x == '-'))
            {
                StringBuilder stringBuilder= new StringBuilder();
                for (int i = 0;
                     i < numbers.Count;
                     i++)
                {
                    if (numbers[i] == '-')
                    {
                        stringBuilder.Append("-");
                        stringBuilder.Append(numbers[++i]);
                        stringBuilder.Append(", ");
                    }    
                }

                throw new Exception(string.Format("negatives {0} not allowed",stringBuilder.ToString()));
            }

            var sum = numbers.Sum(x => (int)Char.GetNumericValue(x));

            return sum;
        }
    }

测试：

 [TestFixture]
    public class StringCalculatorTests
    {
        [Test]
        public void ShouldReturnZeroForEmptyString()
        {
            var sut = new StringCalculator();
            var result = sut.AddNumbers("");
            Assert.AreEqual(0,result);
        }

        [Test]
        [TestCase(1,"1")]
        [TestCase(2,"2")]
        public void ShouldReturnNumberIfGivenOneNumber(int expected,string arg)
        {
            var sut = new StringCalculator();
            var result = sut.AddNumbers(arg);
            Assert.AreEqual(expected,result);
        }

        [Test]
        [TestCase(3, "1,2")]
        [TestCase(11, "1,2,3,5")]
        public void ShouldReturnSumOfAllNumbers(int expected, string arg)
        {
            var sut = new StringCalculator();
            var result = sut.AddNumbers(arg);
            Assert.AreEqual(expected, result);
        }

        [Test]
        [TestCase(3, "1\n2")]
        [TestCase(11, "1,2\n3,5")]
        public void ShouldAllowNewLineAsASeparator(int expected, string arg)
        {
            var sut = new StringCalculator();
            var result = sut.AddNumbers(arg);
            Assert.AreEqual(expected, result);
        }

        [Test]
        [TestCase(3, "//;\n1;2")]
        [TestCase(11, "//.\n1.2\n3.5")]
        [TestCase(11, "//-\n1-2\n3-5")]
        public void ShouldSupportDifferentSeparators(int expected, string arg)
        {
            var sut = new StringCalculator();
            var result = sut.AddNumbers(arg);
            Assert.AreEqual(expected, result);
        }

        [Test]
        [TestCase(3, "//;\n-1;2")]
        [TestCase(3, "//;\n-1;-2")]
        [ExpectedException(typeof(Exception))]
        public void ShouldThrowExceptionIfNegativeInArgs(int expected, string arg)
        {
            var sut = new StringCalculator();
            var result = sut.AddNumbers(arg);
            Assert.AreEqual(expected, result);
        }
    }

Answer 1

单责任原则

方法AddNumbers()违反SRP，因为它是

• 解析参数
• 组合异常消息
• 加数字

这应该通过不同的方法来完成，比如

private bool ContainsCustomDelimiter(string argument)
private string GetCustomDelemiter(string argument)
private string ComposeExceptionMessage(IEnumerable<char> numbers)  

通过将此方法拆分为具有定义责任的较小方法，您的代码将更易于维护和扩展。

号码还是数字?

这是最困扰我的问题。我想到了像1，3，12，56这样的数字，但是看起来这个方法只会和分开的数字相加，这些数字应该在文档中清楚地表示出来。

测试

testname ShouldReturnZeroForEmptyString()是误导性的，或者AddNumbers()方法不返回预期的结果。

该方法可以接受0、1或2个数字，并返回它们的和(对于空字符串它将返回0)，例如“1”或“1”或“1，2”。

但是空字符串不是空字符串，但是AddNumbers()方法返回空值的0也是因为

if(string.IsNullOrEmpty(args))  

在我看来，如果传入的字符串是AddNumbers()，那么NullReferenceException方法应该抛出一个null。

所以你最好检查一下

if (args.Length == 0) { return 0; } 

如果args是null，它将引发NRE。

但还有另一个大问题。假设您将下列字符串之一传递给AddNumbers()方法：

• "/“->抛出IndexOutOfRange
• "//“->抛出IndexOutOfRange

您需要始终检查这样的边缘情况，这就是测试的目的。

现在让我们来谈谈输入无效的情况。

对于给定的字符串，如"1, 2，3“或"1，2，A”，应该发生什么？第一个将返回5，第二个也将返回5个。

因此，您最好检查numbers是否包含任何非数字，并处理此情况。

命名

命名很重要，因为它可以告诉您(如果正确的话)变量是关于什么的。如果他/她看到一个名为sut的变量(但他/她不知道(和我一样) sut代表“系统测试中的系统”)，他/她就会很难过。不过，你为什么不把它命名为calculator呢？

Answer 2

脑海中浮现的一些东西是：

• 用var numbers = args.ToCharArray().Where(x => !delimeters.Contains(x)).ToList();替换var numbers = args.Where(x => !delimeters.Contains(x));，除非您特别想要一个列表。ToCharArray()调用应该是多余的。
• 为了可读性起见，用if (numbers.Any(x => x == '-'))代替if (numbers.Contains('-'))。
• 命令stringBuilder.Append(numbers[++i]);使其在每个循环中增加两次i。是故意的吗？如果没有，您应该删除额外的++并用i + 1替换它。

除此之外，我在代码中没有看到任何问题。

Answer 3

除了已经说过的内容之外，您还应该考虑更多地考虑您的测试套件(我假设您在最近的版本中使用NUnit )。

将干的应用于测试

在所有测试中，您有两行大致相同的行：

var sut = new StringCalculator();
var result = sut.AddNumbers(...);

将计算器实例声明为夹具中的字段，并在[SetUp]方法中进行初始化。

还有var result = ...的复制。在这种特殊情况下，我可能会彻底消除result：

public void ShouldXxx(int expected,string arg)
{
    Assert.AreEqual(expected, calculator.AddNumbers(arg));
}

在我看来，这篇文章读得更好，IMHO：

public void ShouldXxx(int expected, string arg)
{
    Assert.That(calculator.AddNumbers(arg), Is.EqualTo(expected));
}

试着为上面的每一个示例大声朗读代码。

此外，在最新版本中添加几行中断，可以使您在视觉上将测试的内容与预期的结果分离开来，但这是一个非常个人的观点。参见下面重构的示例。

不使用SUT字面意思是

正如其他人已经说过的，不要使用sut作为变量名。当然，您可能了解上下文并熟悉缩略语，但我仍然认为calculator.AddNumbers(...)比sut.AddNumbers()更有表现力。

可以使用例如sut作为变量名的一种情况是，您有一个非常通用的测试套件，可以在许多实现中重用。但是，即使这样，我仍然强烈建议您根据在特定套件中测试的功能类型来命名引用SUT的变量。一个简短的例子：

public abstract class CloneableTests<T> where T: ICloneable
{
    private T cloneable; // Not 'sut'
    ....
}

删除不需要的属性

您不需要使用[TestFixture]属性。用[SetUp]或其中一个[Test...]属性标记单个方法就足够了。同时，当[Test]存在时使用[TestCase]也是多余的。

下面是重构测试的摘录，并进行了一些重新格式化的测试：

public class StringCalculatorTests
{
    private StringCalculator calculator;

    [SetUp]
    public void InitFixture()
    {
        calculator = new StringCalculator();
    }

    [Test]
    public void ShouldReturnZeroForEmptyString()
    {
        Assert.That(
            calculator.AddNumbers(String.Empty),
            Is.EqualTo(0)
        );
    }

    [TestCase(1, "1")]
    [TestCase(2, "2")]
    public void ShouldReturnNumberIfGivenOneNumber(int expected, string arg)
    {
        Assert.That(
            calculator.AddNumbers(arg),
            Is.EqualTo(expected)
        );
    }

    ...
}