Converting a range of integers from a string to an IEnumerable

Question

Goal

Convert the following into an IEnumerable of integers accounting for the x-y ranges:

"1,2,3,0,7,8,9,10-15"

to

{ 1, 2, 3, 0, 7, 8, 9, 10, 11, 12, 13, 14, 15 }

Current implementation

I've written this little function and got something working, but I'm wondering if this can become more efficient or rich in features. My test cases all pass which cover positive and negative ranges.

public IEnumerable<int> GetRange(string numbers) {
    string[] items = numbers.Split(',');

    foreach (var item in items) {
        if (!string.IsNullOrWhiteSpace(item)) {
            //does it contain a -? it's a range then
            int result;
            if (item.Contains("-") && !item.EndsWith("-")) {
                int start, end;
                if (int.TryParse(item.Substring(0, item.IndexOf("-")), out start) && int.TryParse(item.Substring(item.IndexOf("-") + 1), out end)) {
                    int direction = start < end ? 1 : -1;
                    for (result = start; (direction == 1 ? result < end + direction : result > end + direction); result += direction)
                        yield return result;

                    continue;
                }
            }

            if (int.TryParse(item, out result)) {
                yield return result;
                continue;
            }

        }

        throw new InvalidCastException(string.Format("Unable to cast \"{0}\" to an int", item));
    }
}

Answer 1

Ok, some comments:

• Code style
  • While there are no definitive laws about code style, most C# programmers are putting the opening brace { on a separate line. If someone else is going to read your code, they are likely to be annoyed.
  • Consider breaking up the method into smaller methods to clarify how it works, which is better than adding comments.

• Nesting, minimizing nesting makes the code easier to read and in this case you can certainly remove a few levels, namely:

  • if (!string.IsNullOrWhiteSpace(item)) can be removed if you instead split the string with the parameter StringSplitOptions.RemoveEmptyEntries.
  • If the number cannot be parsed, you could throw the exception when TryParse returns false and keep the "happy path" code to run whenever TryParse is successful.

• Error handling

  • Since it is TryParse that is failing, you should probably throw a FormatException to indicate invalid format or ArgumentException instead of InvalidCastException to indicate invalid input parameter. Since there is no casting that is failing within the code, this may confuse other developers when debugging the code.
  • Stricter validation of input data, for example, to handle the case with the method being invoked with the numbers parameter set to null.

• Performance

  • There is always things to improve in this department, however, if the method is fast enough for you, I suggest focusing on readability.
• Defect
  • Invoking the method with a negative range where the first number is negative will not work.