How to make my go parser code more readable

Question

I'm writing a recursive descent parser in Go for a simple made-up language, so I'm designing the grammar as I go. My parser works but I wanted to ask if there are any best practices for how I should lay out my code or when I should put code in its own function etc ... to make it more readable.

I've been building the parser by following the simple rules I've learned so far ie. each non-terminal is it's own function, even though my code works I think looks really messy and unreadable.

I've included the code for the assignment non-terminal and the grammar above the function.

I've taken out most of the error handling to keep the function smaller.

Here's some examples of what that code can parse:

a = 10
a,b,c = 1,2,3
a int = 100
a,b string = "hello", "world"

Can anyone give me some advice as to how I can make my code more readable please?

// assignment                 : variable_list '=' expr_list
//                            | variable_list type
//                            | variable_list type '=' expr_list
func (p *Parser) assignment() ast.Noder {

    assignment := &ast.AssignmentNode{}

    assignment.Left = p.variable_list()

    // This if-statement deals with rule 2 or 3
    if p.currentToken.Type != token.ASSIGN {
        // Static variable declaration
        // Could be a declaration or an assignment
        // Only static variables can be declared without providing a value
        assignment.IsStatic = true

        assignment.Type = p.var_type().Value

        assignment.Right = nil

        p.nextToken()
        // Rule 2 is finished at this point in the code
        // This if-statement is for rule 3
        if p.currentToken.Type == token.ASSIGN {
            assignment.Operator = p.currentToken

            p.nextToken()

            assignment.Right = p.expr_list()
        }

    } else {
        // This deals with rule 1
        assignment.Operator = p.currentToken

        p.nextToken()

        assignment.Right = p.expr_list()

    }

    if assignment.Right == nil {
        for i := 0; i < len(assignment.Left); i++ {
            assignment.Right = append(assignment.Right, nil)
        }
    }

    if len(assignment.Left) != len(assignment.Right) {
        p.FoundError(p.syntaxError("variable mismatch, " + strconv.Itoa(len(assignment.Left)) + " on left but " + strconv.Itoa(len(assignment.Right)) + " on right,"))
    }

    return assignment
}

Answer 1

how I can make my code more readable?

---

For readability, a prerequisite for correct, maintainable code,

// assignment                 : variable_list '=' expr_list
//                            | variable_list type
//                            | variable_list type '=' expr_list
func (p *Parser) assignment() ast.Noder {
    assignment := &ast.AssignmentNode{}

    // variable_list
    assignment.Left = p.variable_list()

    // type
    if p.currentToken.Type != token.ASSIGN {
        // Static variable declaration
        // Could be a declaration or an assignment
        // Only static variables can be declared without providing a value
        assignment.IsStatic = true

        assignment.Type = p.var_type().Value
        p.nextToken()
    }

    // '=' expr_list
    if p.currentToken.Type == token.ASSIGN {
        assignment.Operator = p.currentToken
        p.nextToken()
        assignment.Right = p.expr_list()
    }

    // variable_list [expr_list]
    if assignment.Right == nil {
        for i := 0; i < len(assignment.Left); i++ {
            assignment.Right = append(assignment.Right, nil)
        }
    }
    if len(assignment.Left) != len(assignment.Right) {
        p.FoundError(p.syntaxError(fmt.Sprintf(
            "variable mismatch, %d on left but %d on right,",
            len(assignment.Left), len(assignment.Right),
        )))
    }

    return assignment
}

---

Note: This likely inefficient and overly complicated:

for i := 0; i < len(assignment.Left); i++ {
    assignment.Right = append(assignment.Right, nil)
}

What is the type of assignment.Right?

Answer 2

As far as how to make your code more readable, there is not always a cut and dry answer. I personally find that code is more readable when you can use function names in place of comments in the code. A lot of people like to recommend the book "Clean Code" by Robert C. Martin. He pushes this throughout the book, small functions that have one purpose and are self documenting (via the function name).

Of course, as I said before this is a subjective topic. I took a crack at it, and came up with the code below, which I personally feel is more readable. It also uses the function names to document what is going on. That way the reader doesn't necessarily need to dig into every single statement in the code, but rather just the high level function names if they don't need all of the details.

// assignment                 : variable_list '=' expr_list
//                            | variable_list type
//                            | variable_list type '=' expr_list
func (p *Parser) assignment() ast.Noder {
    assignment := &ast.AssignmentNode{}
    assignment.Left = p.variable_list()

    // This if-statement deals with rule 2 or 3
    if p.currentToken.Type != token.ASSIGN {
        // Static variable declaration
        // Could be a declaration or an assignment
        // Only static variables can be declared without providing a value
        p.parseStaticStatement(assignment)
    } else {
        p.parseVariableAssignment(assignment)
    }

    if assignment.Right == nil {
        assignment.appendDefaultValues()
    }

    p.checkForUnbalancedAssignment(assignment)

    return assignment
}

func (p *Parser) parseStaticStatement(assignment *ast.AssingmentNode) {
    assignment.IsStatic = true
    assignment.Type = p.var_type().Value
    assignment.Right = nil

    p.nextToken()

    // Rule 2 is finished at this point in the code
    // This if-statement is for rule 3
    if p.currentToken.Type == token.ASSIGN {
        a.parseStaticAssignment()
    } 
}

func (p *Parser) parseStaticAssignment(assignment *ast.AssignmentNode) {
    assignment.Operator = p.currentToken
    p.nextToken()
    assignment.Right = p.expr_list() 
}

func (p *Parser) parseVariableAssignment(assignment *ast.AssignmentNode) {
    // This deals with rule 1
    assignment.Operator = p.currentToken
    p.nextToken()
    assignment.Right = p.expr_list()
}

func (a *ast.AssignmentNode) appendDefaultValues() {
    for i := 0; i < len(assignment.Left); i++ {
        assignment.Right = append(assignment.Right, nil)
    }
}

func (p *Parser) checkForUnbalancedAssignment(assignment *ast.AssignmentNode) {
    if len(assignment.Left) != len(assignment.Right) {
        p.FoundError(p.syntaxError("variable mismatch, " + strconv.Itoa(len(assignment.Left)) + " on left but " + strconv.Itoa(len(assignment.Right)) + " on right,"))
    }
}

I hope that you find this helpful. I am more than willing to answer any further questions that you may have if you leave a comment on my response.