5
\$\begingroup\$

I have posted this question 2 days ago and got amazing feedback on it.

Hands back to the keyboard, I believe I took the suggestions into account and improved the classes drastically. In this question, would love further criticism and suggestions in regard to my code logic and coding style.

The TokenFactory and TokenParser:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

namespace ShirLanguageCompiler
{
    public class TokenParser
    {
        private Regex Pattern;
        private SyntaxKind Type;
        public bool CanParse(TokenFactory factory) => Pattern.IsMatch(factory.UpcomingCode);

        public Token Parse(TokenFactory factory)
        {
            Match match = Pattern.Match(factory.UpcomingCode);

            return new Token(factory.Position, match.Length, factory.Location, Type, match.Value);
        }
        public TokenParser(Regex pattern, SyntaxKind type)
        {
            this.Pattern = pattern;
            this.Type = type;
        }
    }
    public class TokenFactory
    {
        private int Length { get; }
        private string Code { get; set; }

        public int Position { get; set; }
        public (int Line, int Column) Location;
        public string UpcomingCode => this.Code.Substring(Position);

        TokenParser[] Parsers = {
            new TokenParser(new Regex(@"^""(?:\\""|[^""])*""",  RegexOptions.Compiled),SyntaxKind.LiteralStringToken        ),
            new TokenParser(new Regex(@"^'(?:\\'|[^'])+'",      RegexOptions.Compiled),SyntaxKind.LiteralCharToken          ),
            new TokenParser(new Regex(@"^d\d+",                 RegexOptions.Compiled),SyntaxKind.LiteralNumberToken        ),
            new TokenParser(new Regex(@"^false",                RegexOptions.Compiled),SyntaxKind.LiteralFalseKeyword       ),
            new TokenParser(new Regex(@"^true",                 RegexOptions.Compiled),SyntaxKind.LiteralTrueKeyword        ),

            new TokenParser(new Regex(@"^:",                    RegexOptions.Compiled),SyntaxKind.ColonToken                ),
            new TokenParser(new Regex(@"^;",                    RegexOptions.Compiled),SyntaxKind.SemiColonToken            ),

            new TokenParser(new Regex(@"^\s",                   RegexOptions.Compiled),SyntaxKind.WhitespaceToken           ),
            new TokenParser(new Regex(@"^[\r\n]+",              RegexOptions.Compiled),SyntaxKind.EOLToken                  ),

            new TokenParser(new Regex(@"^,",                    RegexOptions.Compiled),SyntaxKind.CommaToken                ),
            new TokenParser(new Regex(@"^->",                   RegexOptions.Compiled),SyntaxKind.AccessorToken             ),
            new TokenParser(new Regex(@"^=>",                   RegexOptions.Compiled),SyntaxKind.AssignmentToken           ),

            new TokenParser(new Regex(@"^\(",                   RegexOptions.Compiled),SyntaxKind.OpenParenthesisToken      ),
            new TokenParser(new Regex(@"^\)",                   RegexOptions.Compiled),SyntaxKind.CloseParenthesisToken     ),
            new TokenParser(new Regex(@"^\{",                   RegexOptions.Compiled),SyntaxKind.OpenCurlyBracketToken     ),
            new TokenParser(new Regex(@"^\)}",                  RegexOptions.Compiled),SyntaxKind.CloseCurlyBracketToken    ),
            new TokenParser(new Regex(@"^\[",                   RegexOptions.Compiled),SyntaxKind.OpenSquareBracketToken    ),
            new TokenParser(new Regex(@"^\]",                   RegexOptions.Compiled),SyntaxKind.CloseSquareBracketToken   ),

            new TokenParser(new Regex(@"^\?",                   RegexOptions.Compiled),SyntaxKind.QuestionMarkToken         ),
            new TokenParser(new Regex(@"^\+",                   RegexOptions.Compiled),SyntaxKind.PlusOperationToken        ),
            new TokenParser(new Regex(@"^\-",                   RegexOptions.Compiled),SyntaxKind.MinusOperationToken       ),
            new TokenParser(new Regex(@"^\*}",                  RegexOptions.Compiled),SyntaxKind.MultiplyOperationToken    ),
            new TokenParser(new Regex(@"^\*\*",                 RegexOptions.Compiled),SyntaxKind.PowerOperationToken       ),
            new TokenParser(new Regex(@"^\/",                   RegexOptions.Compiled),SyntaxKind.DivideOperationToken      ),
            new TokenParser(new Regex(@"^\/\/",                 RegexOptions.Compiled),SyntaxKind.RootOperationToken        ),
            new TokenParser(new Regex(@"^==",                   RegexOptions.Compiled),SyntaxKind.EqualToken                ),
            new TokenParser(new Regex(@"^!=",                   RegexOptions.Compiled),SyntaxKind.InEqualToken              ),

            new TokenParser(new Regex(@"^letter",               RegexOptions.Compiled),SyntaxKind.LetterKeyword             ),
            new TokenParser(new Regex(@"^number",               RegexOptions.Compiled),SyntaxKind.NumberKeyword             ),
            new TokenParser(new Regex(@"^boolean",              RegexOptions.Compiled),SyntaxKind.BooleanKeyword            ),
            new TokenParser(new Regex(@"^str",                  RegexOptions.Compiled),SyntaxKind.StringKeyword             ),

            new TokenParser(new Regex(@"^bind",                 RegexOptions.Compiled),SyntaxKind.BindKeyword               ),
            new TokenParser(new Regex(@"^return",               RegexOptions.Compiled),SyntaxKind.ReturnKeyword             ),

            new TokenParser(new Regex(@"^ref",                   RegexOptions.Compiled),SyntaxKind.RefKeyword               ),
            new TokenParser(new Regex(@"^val",                   RegexOptions.Compiled),SyntaxKind.ValKeyword               ),
            new TokenParser(new Regex(@"^\D\w+\(\)",             RegexOptions.Compiled),SyntaxKind.FunctionNameToken        ),
            new TokenParser(new Regex(@"^\D\w+",                RegexOptions.Compiled),SyntaxKind.VariableNameToken        )
        };


        public TokenFactory(string code)
        {
            this.Code = code;
            this.Length = code.Length;
            this.Position = 0;
            this.Location = (0, 0);
        }

        public IEnumerable<Token> GetTokenStream()
        {
            while(this.Position < this.Length)
            {
                TokenParser Parser = Parsers.First(n => n.CanParse(this));
                Token Parsed = Parser.Parse(this);
                if (Parsed.Type == SyntaxKind.EOLToken)
                    this.Location = (this.Location.Line++,0);
                else
                    this.Location.Column += Parsed.Length;
                this.Position += Parsed.Length;

                #if DEBUG
                    Console.Write(Parsed);
                #endif
                    yield return Parsed;
            }
        }

    }
}

And here the Token class:

using System.Diagnostics;
using System.Text.RegularExpressions;

namespace ShirLanguageCompiler
{
    public enum SyntaxKind
    {
        PlusOperationToken,
        MinusOperationToken,
        DivideOperationToken,
        MultiplyOperationToken,
        PowerOperationToken,
        RootOperationToken,

        EqualToken,
        InEqualToken,

        VariableNameToken,
        FunctionNameToken,

        NumberKeyword,
        BooleanKeyword,
        LetterKeyword,
        StringKeyword,

        BindKeyword,
        ReturnKeyword,
        RefKeyword,
        ValKeyword,

        LiteralTrueKeyword,
        LiteralFalseKeyword,

        LiteralNumberToken,
        LiteralCharToken,
        LiteralStringToken,

        EOLToken,
        QuoteToken,
        ColonToken,
        SemiColonToken,
        CommaToken,
        QuestionMarkToken,
        WhitespaceToken,

        AssignmentToken,
        AccessorToken,

        OpenParenthesisToken,
        CloseParenthesisToken,

        OpenCurlyBracketToken,
        CloseCurlyBracketToken,

        OpenSquareBracketToken,
        CloseSquareBracketToken
    }
    public interface IContainsLocation
    {
        (int Line, int Column) Location { get; set; }
        string LocationString { get; }
    }

    [DebuggerDisplay("Value = {EscapedString} Start = {Start}, Length = {Length}, Location = {LocationString}")]
    public class Token: IContainsLocation
    {

        public int Start, Length;
        public (int Line, int Column) Location { get; set; }
        public SyntaxKind Type { get; private set; }
        public string InnerValue { get; private set; }

        public string LocationString
        {
            get
            {
                return $"(Line:{this.Location.Line},Collumn:{this.Location.Column})";
            }
        }
        public string EscapedString
        {
            get
            {
                return Regex.Escape(InnerValue);
            }
        }
        public Token(int start, int length, (int Line, int Column) Location, SyntaxKind type, string innervalue)
        {
            this.Start = start;
            this.Length = length;
            this.Type = type;
            this.InnerValue = innervalue;
            this.Location = Location;
        }

        public override string ToString() => this.InnerValue;

    }

}
\$\endgroup\$
3
\$\begingroup\$

I would start by replacing the tuple (int Line, int Column) location with an immutable struct named SourceLocation.

Here are some of the reasons why I would do this.

  1. The concept already exists in several places. TokenFactory, Token, and IContainsLocation know about this tuple. Adding a FileName field to the tuple would require changes to all three types.
  2. TokenParser and Token should not be modifying the contents of this tuple.
  3. TokenFactory modifies its Location field but it is easy to change the interface so that it doesn't need to.
    Replace
    public (int Line, int Column) Location;
    with
    public int Line {get; private set;}
    public int Column {get; private set;}
    Why Properties Matter gives a detailed explanation of why you should avoid using public fields.
  4. Mutable tuples are a relatively new idea and there isn't a consensus on whether they are appropriate to use in APIs. I use them only in types that are internal or private.

Here is how I would write SourceLocation. I used ReSharper to generate the overrides and the IEquatable implementation.

    public struct SourceLocation : IEquatable<SourceLocation>
    {
        public int Line { get; }

        public int Column { get; }

        public SourceLocation(int line, int column)
        {
            Line = line;
            Column = column;
        }

        public bool Equals(SourceLocation other)
        {
            return Line == other.Line && Column == other.Column;
        }

        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj)) return false;
            return obj is SourceLocation && Equals((SourceLocation) obj);
        }

        public override int GetHashCode()
        {
            unchecked
            {
                return (Line * 397) ^ Column;
            }
        }

        public void Deconstruct(out int line, out int column)
        {
            line = Line;
            column = Column;
        }
    }

TokenParser

  • Pattern and Type should both be marked readonly.

  • Your code currently evaluates each regular expression twice.
    Consider changing the interface to something like this:
    public bool TryParse(TokenFactory factory, out Token token)

  • TokenParser knows more about TokenFactory than it needs to. If you change the signature of TryParse to the following then you can test it independently of TokenFactory.
    public bool TryParse(int position, string input, SourceLocation location, out Token token)
    This would make it easier to write unit tests for the different SyntaxKinds and run them on every build.

Token

  • This class seems like it could be made immutable without any loss in functionality.
  • You can make Token immutable by replacing the public fields with get-only properties, removing all other property setters, and sealing the class. Immutability eliminates the need to reason about what state an object is in. The article Functional C#: Immutability discusses this topic in detail.
\$\endgroup\$
  • \$\begingroup\$ Great suggestions and articles to read up on, thank you! \$\endgroup\$ – downrep_nation Jun 18 '17 at 6:59

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.