Return to Answer

Couple of things I would do to the AttributeParser abstract factory:

1. Bring the creation of the Regex out of the Parse method since it is the same each call.

2. The if..else each with a return in it is a bit wordy. I like the ternary operator just for these instances.

3. Same advice in (1) goes for the Regexes in the ParseFileHeader, ParseDeclarations and ParseMembers methods of the CodeFileParser class.

4. In the ParseDeclarations method, I'd reverse the if (isDeclarationSection) { ... } into if (!isDeclarationSection) { continuel } as I'm a proponent of "fail fast".

5. ParseMembers is one ginormous method. I can't quite tell exactly everything it does. Or, more to the point, it does seem to do everything. I see a lot of if..elses around that look like good break-up areas into other methods - perhaps ParseProperty, ParseLoop, ParseConditional, ParseMethod, etc.

6. I'm going to noodle on the other parts of the code and update this answer in a bit.

The refinagled code:

public class AttributeParser : IAttributeParser
{
    private const string Syntax = @"^Attribute\s(?<Member>[a-zA-Z]+\.)?(?<Name>VB_\w+)\s=\s(?<Value>.*)$";

    private static readonly Regex regex = new Regex(Syntax, RegexOptions.Compiled);

    public IAttribute Parse(string instruction)
    {
        if (!regex.IsMatch(instruction))
        {
            return null;
        }

        var match = regex.Match(instruction);
        var member = match.Groups["Member"].Value;
        var name = match.Groups["Name"].Value;
        var value = match.Groups["Value"].Value;

        return string.IsNullOrEmpty(member)
            ? new Attribute(name, value)
            : new MemberAttribute(name, value, member);
    }
}

Couple of things I would do to the AttributeParser abstract factory:

1. Bring the creation of the Regex out of the Parse method since it is the same each call.

2. The if..else each with a return in it is a bit wordy. I like the ternary operator just for these instances.

3. Same advice in (1) goes for the Regexes in the ParseFileHeader, ParseDeclarations and ParseMembers methods of the CodeFileParser class.

4. In the ParseDeclarations method, I'd reverse the if (isDeclarationSection) { ... } into if (!isDeclarationSection) { continue; } as I'm a proponent of "fail fast".

5. ParseMembers is one ginormous method. I can't quite tell exactly everything it does. Or, more to the point, it does seem to do everything. I see a lot of if..elses around that look like good break-up areas into other methods - perhaps ParseProperty, ParseLoop, ParseConditional, ParseMethod, etc.

6. I'm going to noodle on the other parts of the code and update this answer in a bit.

The refinagled code:

public class AttributeParser : IAttributeParser
{
    private const string Syntax = @"^Attribute\s(?<Member>[a-zA-Z]+\.)?(?<Name>VB_\w+)\s=\s(?<Value>.*)$";

    private static readonly Regex regex = new Regex(Syntax, RegexOptions.Compiled);

    public IAttribute Parse(string instruction)
    {
        if (!regex.IsMatch(instruction))
        {
            return null;
        }

        var match = regex.Match(instruction);
        var member = match.Groups["Member"].Value;
        var name = match.Groups["Name"].Value;
        var value = match.Groups["Value"].Value;

        return string.IsNullOrEmpty(member)
            ? new Attribute(name, value)
            : new MemberAttribute(name, value, member);
    }
}

Couple of things I would do to the AttributeParser abstract factory:

1. Bring the creation of the Regex out of the Parse method since it is the same each call.

2. The if..else each with a return in it is a bit wordy. I like the ternary operator just for these instances.

3. Same advice in (1) goes for the Regexes in the ParseFileHeader, ParseDeclarations and ParseMembers methods of the CodeFileParser class.

4. I'm going to noodle on the other parts of the code and update this answer in a bit.

The refinagled code:

public class AttributeParser : IAttributeParser
{
    private const string Syntax = @"^Attribute\s(?<Member>[a-zA-Z]+\.)?(?<Name>VB_\w+)\s=\s(?<Value>.*)$";

    private static readonly Regex regex = new Regex(Syntax, RegexOptions.Compiled);

    public IAttribute Parse(string instruction)
    {
        if (!regex.IsMatch(instruction))
        {
            return null;
        }

        var match = regex.Match(instruction);
        var member = match.Groups["Member"].Value;
        var name = match.Groups["Name"].Value;
        var value = match.Groups["Value"].Value;

        return string.IsNullOrEmpty(member)
            ? new Attribute(name, value)
            : new MemberAttribute(name, value, member);
    }
}

Couple of things I would do to the AttributeParser abstract factory:

1. Bring the creation of the Regex out of the Parse method since it is the same each call.

2. The if..else each with a return in it is a bit wordy. I like the ternary operator just for these instances.

3. Same advice in (1) goes for the Regexes in the ParseFileHeader, ParseDeclarations and ParseMembers methods of the CodeFileParser class.

4. In the ParseDeclarations method, I'd reverse the if (isDeclarationSection) { ... } into if (!isDeclarationSection) { continuel } as I'm a proponent of "fail fast".

5. ParseMembers is one ginormous method. I can't quite tell exactly everything it does. Or, more to the point, it does seem to do everything. I see a lot of if..elses around that look like good break-up areas into other methods - perhaps ParseProperty, ParseLoop, ParseConditional, ParseMethod, etc.

6. I'm going to noodle on the other parts of the code and update this answer in a bit.

The refinagled code:

public class AttributeParser : IAttributeParser
{
    private const string Syntax = @"^Attribute\s(?<Member>[a-zA-Z]+\.)?(?<Name>VB_\w+)\s=\s(?<Value>.*)$";

    private static readonly Regex regex = new Regex(Syntax, RegexOptions.Compiled);

    public IAttribute Parse(string instruction)
    {
        if (!regex.IsMatch(instruction))
        {
            return null;
        }

        var match = regex.Match(instruction);
        var member = match.Groups["Member"].Value;
        var name = match.Groups["Name"].Value;
        var value = match.Groups["Value"].Value;

        return string.IsNullOrEmpty(member)
            ? new Attribute(name, value)
            : new MemberAttribute(name, value, member);
    }
}

Couple of things I would do to the AttributeParser abstract factory:

1. Bring the creation of the Regex out of the Parse method since it is the same each call.

2. The if..else each with a return in it is a bit wordy. I like the ternary operator just for these instances.

3. I'm going to noodle on the other parts of the code and update this answer in a bit.

The refinagled code:

public class AttributeParser : IAttributeParser
{
    private const string Syntax = @"^Attribute\s(?<Member>[a-zA-Z]+\.)?(?<Name>VB_\w+)\s=\s(?<Value>.*)$";

    private static readonly Regex regex = new Regex(Syntax, RegexOptions.Compiled);

    public IAttribute Parse(string instruction)
    {
        if (!regex.IsMatch(instruction))
        {
            return null;
        }

        var match = regex.Match(instruction);
        var member = match.Groups["Member"].Value;
        var name = match.Groups["Name"].Value;
        var value = match.Groups["Value"].Value;

        return string.IsNullOrEmpty(member)
            ? new Attribute(name, value)
            : new MemberAttribute(name, value, member);
    }
}

Couple of things I would do to the AttributeParser abstract factory:

1. Bring the creation of the Regex out of the Parse method since it is the same each call.

2. The if..else each with a return in it is a bit wordy. I like the ternary operator just for these instances.

3. Same advice in (1) goes for the Regexes in the ParseFileHeader, ParseDeclarations and ParseMembers methods of the CodeFileParser class.

4. I'm going to noodle on the other parts of the code and update this answer in a bit.

The refinagled code:

public class AttributeParser : IAttributeParser
{
    private const string Syntax = @"^Attribute\s(?<Member>[a-zA-Z]+\.)?(?<Name>VB_\w+)\s=\s(?<Value>.*)$";

    private static readonly Regex regex = new Regex(Syntax, RegexOptions.Compiled);

    public IAttribute Parse(string instruction)
    {
        if (!regex.IsMatch(instruction))
        {
            return null;
        }

        var match = regex.Match(instruction);
        var member = match.Groups["Member"].Value;
        var name = match.Groups["Name"].Value;
        var value = match.Groups["Value"].Value;

        return string.IsNullOrEmpty(member)
            ? new Attribute(name, value)
            : new MemberAttribute(name, value, member);
    }
}