3
\$\begingroup\$

I want to read a file containing comma-separated values, so have written a finite state machine:

private IList<string> Split(string line)
{
    List<string> values = new List<string>();
    string value = string.Empty;
    ParseState state = ParseState.Initial;
    foreach (char c in line)
    {
        switch (state)
        {
            case ParseState.Initial:
                switch (c)
                {
                    case COMMA:
                        values.Add(string.Empty);
                        break;
                    case QUOTE:
                        state = ParseState.Quote;
                        break;
                    default:
                        value += c;
                        state = ParseState.Data;
                        break;
                }
                break;
            case ParseState.Data:
                switch (c)
                {
                    case COMMA:
                        values.Add(value);
                        value = string.Empty;
                        state = ParseState.Initial;
                        break;
                    case QUOTE:
                        throw new InvalidDataException("Improper quotes");
                    default:
                        value += c;
                        break;
                }
                break;
            case ParseState.Quote:
                switch (c)
                {
                    case QUOTE:
                        state = ParseState.QuoteInQuote;
                        break;
                    default:
                        value += c;
                        break;
                }
                break;
            case ParseState.QuoteInQuote:
                switch (c)
                {
                    case COMMA:
                        values.Add(value);
                        value = string.Empty;
                        state = ParseState.Initial;
                        break;
                    case QUOTE:
                        value += c;
                        state = ParseState.Quote;
                        break;
                    default:
                        throw new InvalidDataException("Unpaired quotes");
                }
                break;
        }
    }

    switch (state)
    {
        case ParseState.Initial:
        case ParseState.Data:
        case ParseState.QuoteInQuote:
            values.Add(value);
            break;
        case ParseState.Quote:
            throw new InvalidDataException("Unclosed quotes");
    }

    return values;
}

Yes, I know the advice about CSV parsers is "don't write your own", but

  1. I needed it quickly and
  2. our download policy at work would take several days to allow me to get open source off the 'net.

Hey, at least I didn't start with string.Split() or, worse, try using a Regex!

And yes, I know it could be improved by using a StringBuilder, and it's restrictive on quotes in the data, but

  1. performance is not an issue and
  2. this is only to generate well-defined test data in-house,

so I don't care about those.

What I do care about is the apparent trailing block at the end for mopping up all the data after the final comma, and the way that it's starting to look like some sort of an anti-pattern down there, which was exactly the sort of thing that "good" patterns such as a FSM were supposed to avoid.

So my question is this: is this block at the end some sort of anti-pattern, and is it something that's going to come back to bite me in the future?

\$\endgroup\$
  • \$\begingroup\$ There is no need for a question in the title on this site. It's already in the body, so it will be seen. All code here can be reviewed in any way. \$\endgroup\$ – Jamal May 25 '15 at 14:01
2
\$\begingroup\$

So my question is this: is this block at the end some sort of anti-pattern, and is it something that's going to come back to bite me in the future?

Not necessarily. It depends on whether line can contain end-of-line markers.

If line can contain an EOL character that marks the end of the CSV record for you, then you could move the logic from the last switch into case EOL: for the state-specific handling as appropriate, and stop further processing. Or you want to support multi-line CSV records (newline characters embedded within quotes), then you could continue processing the next line.

If the full length of line is expected to be the complete CSV record, then it's fine to not terminate it with an explicit EOL and handle it in the extra switch as you did.

\$\endgroup\$
  • \$\begingroup\$ A useful thought, but .NET's ReadLine (which is what I'm using for simplicity in the caller) trims off the CR/LF. That's not to say that I couldn't add one back on to the end of the string before calling this method, but that'd be going the long way round just to avoid this pattern. \$\endgroup\$ – ClickRick May 25 '15 at 10:24
  • 1
    \$\begingroup\$ No, adding it back doesn't sound good at all. It's good that the full length of line is the record, and your treatment for the end at the end is fine. \$\endgroup\$ – janos May 25 '15 at 10:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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