Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
[TestCase(new[] { 1, 2 }, 1, Result = 2)]
[TestCase(new[] { 1, 2 }, 2, Result = 1)]
[TestCase(new[] { 1, 2, 3 }, 2, Result = 2)]
[TestCase(new[] { 1, 2, 3, 4 }, 2, Result = 2)]
[TestCase(new[] { 1, 2, 3, 4 }, 10, Result = 1)]
public int TotalPageCountIsAccurate(int[] testSequence, int pageSize)
{
    var pagedList = new PagedList<int>(testSequence, 0, pageSize);
    return pagedList.TotalPageCount;
}

I wrote this test about five minutes ago and I can barely understand it - I dread to think how hard it will be to read in a few weeks. How can I refactor the TestCase attribute to improve readability?

I thought about using named arguments or (maybe) introducing a some well-named fields, but neither of these ideas appear to be feasible.

share|improve this question
    
What isn't feasible about using named arguments? –  Jeroen Vannevel Jul 4 at 6:54
    
The TestCase attribute's argument names are along the lines of Argument1, Argument2 etc. which is no more descriptive than not using named arguments. –  ByteBlast Jul 4 at 6:57
    
Oh right. I don't think you can make it any clearer since this is pretty much how it is done. I find it quite readable myself but you can always add some documenttion to describe what the testcases do. –  Jeroen Vannevel Jul 4 at 7:04
add comment

2 Answers 2

up vote 5 down vote accepted

If I'm right you could do at least two things:

  1. Generate the array and pass only the number of items to the test method.

  2. Rename testSequence to something more descriptive, like items, elements etc.

[TestCase(2, 1, Result = 2)]
[TestCase(2, 2, Result = 1)]
[TestCase(3, 2, Result = 2)]
[TestCase(4, 2, Result = 2)]
[TestCase(4, 10, Result = 1)]
public int TotalPageCountIsAccurate(int itemCount, int pageSize)
{
    // generates the item array with itemCount items
    var items = Enumerable.Range(0, itemCount).ToArray();

    var pagedList = new PagedList<int>(items, 0, pageSize);
    return pagedList.TotalPageCount;
}

(I can't check whether is it valid C# or not but I hope you get the idea.)

I would also create a test with an empty array.

share|improve this answer
1  
Tremendous idea. Thanks. –  ByteBlast Jul 4 at 7:29
add comment

I would probably go with @palacsint's idea, but just to throw it out there you can also use a test case factory:

[TestFixture]
public class PagingTests
{
    [Test]
    [TestCaseSource(typeof(PagingTestCaseFactory), "TestCases")]
    public int TotalPageCountIsAccurate(int itemCount, int pageSize)
    {
        var items = Enumerable.Range(0, itemCount).ToArray();

        var pagedList = new PagedList<int>(items, 0, pageSize);

        return pagedList.TotalPageCount;
    }
}

public class PagingTestCaseFactory
{
    public static IEnumerable TestCases
    {
        get
        {
            yield return ItemCountEqualToPageSize(2, 2).Returns(1);
            yield return ItemCountGreaterThanPageSize(2, 1).Returns(2);
            yield return ItemCountGreaterThanPageSize(3, 2).Returns(2);
            yield return ItemCountGreaterThanPageSize(4, 2).Returns(2);
            yield return ItemCountLessThanPageSize(4, 10).Returns(1);
        }
    }

    private static TestCaseData ItemCountEqualToPageSize(int itemCount, int pageSize)
    {
        return new TestCaseData(itemCount, pageSize);
    }

    private static TestCaseData ItemCountGreaterThanPageSize(int itemCount, int pageSize)
    {
        return new TestCaseData(itemCount, pageSize);
    }

    private static TestCaseData ItemCountLessThanPageSize(int itemCount, int pageSize)
    {
        return new TestCaseData(itemCount, pageSize);
    }
}
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.