Detecting interesting car mileages

Question

I've had a go at the following challenge (from codewars):

Interesting car mileages are 3-or-more digit numbers that meet one or more of the following criteria:

• Any digit followed by all zeros: 100, 90000
• Every digit is the same number: 1111
• The digits are sequential, incementing†: 1234
• The digits are sequential, decrementing‡: 4321
• The digits are a palindrome: 1221 or 73837
• The digits match one of the values in the awesomePhrases array

† For incrementing sequences, 0 should come after 9, and not before 1, as in 7890. ‡ For decrementing sequences, 0 should come after 1, and not before 9, as in 3210.

Complete the method IsInteresting, so that it returns:

• 2 if a mileage is interesting
• 1 if an interesting mileage occurs within the next 2 miles
• 0 for any other number

Mileages will be in the range 0 to 1,000,000,000. awesomePhrases will always be supplied, but may be empty.

In order to support development, I wrote the following tests (apart from ShouldWorkTest, which was supplied with the challenge).

using NUnit.Framework;
using System.Collections.Generic;

[TestFixture, Category("MileageChallenge")]
public class CarMileageTests
{
    public const int NotInteresting = 0;
    public const int AlmostInteresting = 1;
    public const int Interesting = 2;

    [TestCase(0)]
    [TestCase(97)]
    //[TestCase(98)]  // Almost Interesting
    //[TestCase(99)]  // Almost Interesting
    public void NumbersBelow100AreNotInteresting(int mileage)
    {
        Assert.AreEqual(NotInteresting, CarMileage.IsInteresting(mileage, new List<int> { }));
    }

    [TestCase(100)]
    [TestCase(7000)]
    [TestCase(900000000)]
    public void NumbersFollowedByOnlyZeroAreInteresting(int mileage)
    {
        Assert.AreEqual(Interesting, CarMileage.IsInteresting(mileage, new List<int> { }));
    }

    [TestCase(102)]
    [TestCase(7700)]
    [TestCase(900700000)]
    public void NumbersIntermingledWithZerosAreNotInteresting(int mileage)
    {
        Assert.AreEqual(NotInteresting, CarMileage.IsInteresting(mileage, new List<int> { }));
    }

    [TestCase(101)]
    [TestCase(7700)]
    [TestCase(900700000)]
    public void NumbersMatchingAwesomeListAreIntersting(int mileage)
    {
        Assert.AreEqual(Interesting, CarMileage.IsInteresting(mileage, new List<int> {600, 101, 900700000, 5555, 7700, 7777 }));
    }


    [TestCase(67576)]
    [TestCase(7117)]
    [TestCase(900000009)]
    public void PalendromesAreInteresting(int mileage)
    {
        Assert.AreEqual(Interesting, CarMileage.IsInteresting(mileage, new List<int> { }));
    }

    [TestCase(34567890)]
    [TestCase(7890)]
    [TestCase(12345)]
    public void IncrementingSequencesAreInteresting(int mileage)
    {
        Assert.AreEqual(Interesting, CarMileage.IsInteresting(mileage, new List<int> { }));
    }

    [TestCase(543210)]
    [TestCase(9876)]
    [TestCase(54321)]
    public void DecrementingSequencesAreInteresting(int mileage)
    {
        Assert.AreEqual(Interesting, CarMileage.IsInteresting(mileage, new List<int> { }));
    }

    // Strictly speaking, special case of palendrome
    [TestCase(4444)]
    [TestCase(99999999)]
    public void AllSameDigitsShouldBeInteresting(int mileage)
    {
        Assert.AreEqual(Interesting, CarMileage.IsInteresting(mileage, new List<int> { }));
    }

    [TestCase(100)]
    [TestCase(900700000)]
    [TestCase(67576)]
    [TestCase(987654)]
    [TestCase(123456)]
    public void CloseMatchesAreAlmostInteresting(int interestingMileage)
    {
        Assert.AreEqual(AlmostInteresting, CarMileage.IsInteresting(interestingMileage - 1, new List<int> { 900700000 }));
        Assert.AreEqual(AlmostInteresting, CarMileage.IsInteresting(interestingMileage - 2, new List<int> { 900700000 }));
    }

    // Supplied by challenge
    [Test]
    public void ShouldWorkTest()
    {
        Assert.AreEqual(0, CarMileage.IsInteresting(3, new List<int>() { 1337, 256 }));
        Assert.AreEqual(1, CarMileage.IsInteresting(1336, new List<int>() { 1337, 256 }));
        Assert.AreEqual(2, CarMileage.IsInteresting(1337, new List<int>() { 1337, 256 }));
        Assert.AreEqual(0, CarMileage.IsInteresting(11208, new List<int>() { 1337, 256 }));
        Assert.AreEqual(1, CarMileage.IsInteresting(11209, new List<int>() { 1337, 256 }));
        Assert.AreEqual(2, CarMileage.IsInteresting(11211, new List<int>() { 1337, 256 }));
    }
}

My solution to the challenge:

using System.Collections.Generic;

public class CarMileage
{

    public static int IsInteresting(int mileage, List<int> awesomePhrases)
    {
        const int NotInteresting = 0;
        const int AlmostInteresting = 1;
        const int Interesting = 2;

        if (IsExactMileageInteresting(mileage, awesomePhrases))
        {
            return Interesting;
        }

        if (   IsExactMileageInteresting(mileage + 1, awesomePhrases) 
            || IsExactMileageInteresting(mileage + 2, awesomePhrases)) 
        {
            return AlmostInteresting;
        }

        return NotInteresting;
    }

    private static bool IsExactMileageInteresting(int mileage, List<int> awesomePhrases)
    {
        if (mileage < 100) return false;

        if (IsSingleDigitAndZeros(mileage)) return true;
        if (IsAwesomePhrase(mileage, awesomePhrases)) return true;

        var mileageString = mileage.ToString();

        if (IsPalendrome(mileageString)) return true;
        if (IsSequence(mileageString, 1)) return true;
        if (IsSequence(mileageString, -1)) return true;

        return false;
    }

    private static bool IsSingleDigitAndZeros(int mileage)
    {
        while(mileage >= 10)
        {
            if (mileage % 10 != 0) return false;
            mileage /= 10;
        }
        return true;
    }

    private static bool IsAwesomePhrase(int mileage, List<int> awesomePhrases)
    {
        return awesomePhrases.Contains(mileage);
    }

    private static bool IsPalendrome(string mileageString)
    {
        int start = 0, end = mileageString.Length-1;

        while(start < end)
        {
            if(mileageString[start] != mileageString[end])
            {
                return false;
            }
            start++;
            end--;
        }

        return true;
    }

    private static bool IsSequence(string mileageString, int increment)
    {
        var previousNumeral = mileageString[0];

        for(var index = 1; index < mileageString.Length; index++)
        {
            if (previousNumeral + increment == mileageString[index]) {
                previousNumeral = mileageString[index];
                continue;
            }
            if(mileageString[index] == '0' &&
              (previousNumeral == '9' || previousNumeral == '1'))
            {
                previousNumeral = mileageString[index];
                continue;
            }
            return false;
        }
        return true;
    }
}

Feedback on any aspect of the tests/solution is welcome.

Answer 1

One thing I'd change would be to have tests for each of the individual methods. Notice how the test has to provide an empty list to test IsPalindrome() or IsSequence()? Palindromes have nothing to do with awesome phrase arrays, yet here they are, adding confusion to the tests.

Isolating the tests will also allow you to test uninteresting hits. Is 101 a hit because it's Awesome, or because it's a palindrome (of course does it really matter, when palindromes are clearly awesome anyway?) But what about 102?

IsSequence() might benefit more from isolated tests, because the intent of the ==9 or ==1 check isn't clear. 7890 is a sequence, but what about 7899? 7891? 78901? What about 890123, or 210987?

I see no error handling. My car has no minus sign, so I doubt -1 is a valid odometer reading. What about 4294967295? 4294967296? I might find it interesting if I could cause an integer overflow or underflow.

IsSequence is kind of confusing because it's handling both increments and decrements. Due to this, it's more difficult to understand how it handles the 0 threshold. Perhaps consider two separate, clearly-named methods that converge on a common logic core.

Consider renaming the class. CarMileage sounds like how much fuel a car uses per mile. Is this more of an AnalyzeOdometerReading or AnalyzeOdometerValue class?

Answer 2

The first thing that strikes me is the repetition of the three constants in the code and in the tests:

    const int NotInteresting = 0;
    const int AlmostInteresting = 1;
    const int Interesting = 2;

I'd make them internal and set the InternalsVisibleTo to the test.

---

IsSequence

For this one I'd use the OrderBy[Descending] + SequenceEqual. This would make it virtually a one-liner.

Answer 3

make the class static

I would return an enum instead of an int

Its Palindrome not Palendrome