views:

66

answers:

3

This class is throwing an exception. It doesn't show me the exact line number, but it sounds like it's occurring in the static constructor:

static class _selectors
{
    public static string[] order = new[] { "ID", "NAME", "TAG" };
    public static Dictionary<string, Regex> match = new Dictionary<string, Regex> {
        { "ID", new Regex(@"#((?:[\w\u00c0-\uFFFF-]|\\.)+)") },
        { "CLASS", new Regex(@"\.((?:[\w\u00c0-\uFFFF-]|\\.)+)") },
        { "NAME", new Regex(@"\[name=['""]*((?:[\w\u00c0-\uFFFF-]|\\.)+)['""]*\]") },
        { "ATTR", new Regex(@"\[\s*((?:[\w\u00c0-\uFFFF-]|\\.)+)\s*(?:(\S?=)\s*(['""]*)(.*?)\3|)\s*\]") },
        { "TAG", new Regex(@"^((?:[\w\u00c0-\uFFFF\*-]|\\.)+)") },
        { "CHILD", new Regex(@":(only|nth|last|first)-child(?:\((even|odd|[\dn+-]*)\))?") },
        { "POS", new Regex(@":(nth|eq|gt|lt|first|last|even|odd)(?:\((\d*)\))?(?=[^-]|$)") },
        { "PSEUDO", new Regex(@":((?:[\w\u00c0-\uFFFF-]|\\.)+)(?:\((['""]?)((?:\([^\)]+\)|[^\(\)]*)+)\2\))?") }
    };
    public static Dictionary<string, Action<HashSet<XmlNode>, string>> relative = new Dictionary<string, Action<HashSet<XmlNode>, string>> {
        { "+", (checkSet, part) => {
        }}
    };
    public static Dictionary<string, Regex> leftMatch = new Dictionary<string, Regex>();
    public static Regex origPOS = match["POS"];

    static _selectors()
    {
        foreach (var type in match.Keys)
        {
            _selectors.match[type] = new Regex(match[type].ToString() + @"(?![^\[]*\])(?![^\(]*\))");
            _selectors.leftMatch[type] = new Regex(@"(^(?:.|\r|\n)*?)" + Regex.Replace(match[type].ToString(), @"\\(\d+)", (m) =>
                @"\" + (m.Index + 1)));
        }
    }
}

Why can't I change those values in the c'tor?

+1  A: 

Simple diagnostic approach: Move all that code into normal methods, and find out what exception is being thrown that way. Or just run it in the debugger - that should break when the exception is thrown.

I suspect it'll be a bad regular expression or something like that.

Personally, I think this is too much logic in a static constructor, but that's a slightly different matter... Note that you're also relying on the ordering of the initialization here:

public static Regex origPOS = match["POS"];

That is guaranteed to be okay at the moment, but it's very brittle.

Jon Skeet
Well, I do need a copy of `match["POS"]` before it's modified in the c'tor... where would you suggest I do that then?
Mark
@Mark: I would probably refactor most of the work into methods in a separate class. That means you can easily unit test them. You could *then* potentially use them from the static constructor.
Jon Skeet
+3  A: 

If you view the inner exception, you will see that it states

Collection was modified; enumeration operation may not execute.

This means you are changing the collection you are looping, which is not allowed.

Rather change your constructor to something like

static _selectors()
{
    List<string> keys = match.Keys.ToList();
    for (int iKey = 0; iKey < keys.Count; iKey++)
    {
        var type = keys[iKey];
        _selectors.match[type] = new Regex(match[type].ToString() + @"(?![^\[]*\])(?![^\(]*\))");
        _selectors.leftMatch[type] = new Regex(@"(^(?:.|\r|\n)*?)" + Regex.Replace(match[type].ToString(), @"\\(\d+)", (m) =>
            @"\" + (m.Index + 1)));
    }
}
astander
Actually, if you `ToList()` it, you can skip the `for` and just enumerate the result of that call. Also, you're calling `ToList()` a crapton here, not exactly the best code.
Will
Yes, that is true, and the *crap ton* to list can be moved to outside of the loop X-)
astander
+1  A: 

You're modifying a collection while enumerating it. You can't do that. Quick fix is to move the keys into a different collection and enumerate that:

static _selectors()
{
    foreach (var type in match.Keys.ToArray())
    {

Also, if you had checked the inner exception you would have seen that was the case.

Will
Inner exception says "Collection was modified; enumeration operation may not execute". I guess that should have tipped me off, but I must not have read it clearly.
Mark
@Mark: In future, if you've *got* the inner exception message, please include it in the question. I still think you should move some of this logic outside the type initializer though :)
Jon Skeet
@Jon The list of stuff he should do with that code is pretty long...
Will
@Will: It's almost a direct port of jQuery... blame them for weird code :p The dictionaries actually do come in handy though, because I need to access those regexes with a variable key.
Mark