Questions
Let's look at the second question first:
Or somehow make that first branch clean?
It appears that you already started to analyze that lex
function. At 137 lines it is a massive function to read through, let alone try updating. Think of anyone reading the code, including your future self. It would be wise to break that function up into smaller functions. This will also help with testing.
One option would be to use OOP - even if only one instance of an object is needed that object could hold the state in properties instead of referencing variables in a large function. Then that monstrous lex
function can be broken up into smaller methods that use properties to update the state.
Is there anything repetitive which could be abstracted out or simplified?
Yes there are quite a few repetitive blocks. For example, the following two blocks appear both within the block starting on line 45 (i.e. if (str.match(/^ *$/)) {
) as well as just before the return statement:
while (nesting > 1) { tokens.push({ form: 'close-paren' }) nesting-- } while (indents.length) { tokens.push({ form: 'close-paren' }) indents.pop() }
Those could be re-written as for
loops. For example:
for (; nesting > 1; nesting--) {
tokens.push({
form: 'close-paren'
})
}
for (; indents.length; indents.pop()) {
tokens.push({
form: 'close-paren'
})
}
Additionally, the following three lines appear eight times:
tokens.push({ form: 'close-paren' })
Making a separate function/method for that seems excessive (since it would lead to additional function calls just to call the push method on an array). It could be declared once before the function:
const closeParenToken = {
form: 'close-paren'
};
And then referenced whenever such a token needs to be pushed into the tokens. If having the same object copied each time is an issue then various techniques could be used - e.g. spreading into a new object - {...closeParenToken}
Other review points
Most variables are declared using let
. Some could be could be declared with const
- e.g. indents
, match
on line 41, match
on line 62, newIndent
, oldIndent
, etc. It is wise to default to using const, unless you know re-assignment is necessary - then use let. This will help avoid accidental re-assignment and other bugs .
Let's look at the second question first:
Or somehow make that first branch clean?
It appears that you already started to analyze that lex
function. At 137 lines it is a massive function to read through, let alone try updating. Think of anyone reading the code, including your future self. It would be wise to break that function up into smaller functions. This will also help with testing.
One option would be to use OOP - even if only one instance of an object is needed that object could hold the state in properties instead of referencing variables in a large function. Then that monstrous lex
function can be broken up into smaller methods that use properties to update the state.
Is there anything repetitive which could be abstracted out or simplified?
Yes there are quite a few repetitive blocks. For example, the following two blocks appear both within the block starting on line 45 (i.e. if (str.match(/^ *$/)) {
) as well as just before the return statement:
while (nesting > 1) { tokens.push({ form: 'close-paren' }) nesting-- } while (indents.length) { tokens.push({ form: 'close-paren' }) indents.pop() }
Those could be re-written as for
loops. For example:
for (; nesting > 1; nesting--) {
tokens.push({
form: 'close-paren'
})
}
for (; indents.length; indents.pop()) {
tokens.push({
form: 'close-paren'
})
}
Additionally, the following three lines appear eight times:
tokens.push({ form: 'close-paren' })
Making a separate function/method for that seems excessive (since it would lead to additional function calls just to call the push method on an array). It could be declared once before the function:
const closeParenToken = {
form: 'close-paren'
};
And then referenced whenever such a token needs to be pushed into the tokens. If having the same object copied each time is an issue then various techniques could be used - e.g. spreading into a new object - {...closeParenToken}
Questions
Let's look at the second question first:
Or somehow make that first branch clean?
It appears that you already started to analyze that lex
function. At 137 lines it is a massive function to read through, let alone try updating. Think of anyone reading the code, including your future self. It would be wise to break that function up into smaller functions. This will also help with testing.
One option would be to use OOP - even if only one instance of an object is needed that object could hold the state in properties instead of referencing variables in a large function. Then that monstrous lex
function can be broken up into smaller methods that use properties to update the state.
Is there anything repetitive which could be abstracted out or simplified?
Yes there are quite a few repetitive blocks. For example, the following two blocks appear both within the block starting on line 45 (i.e. if (str.match(/^ *$/)) {
) as well as just before the return statement:
while (nesting > 1) { tokens.push({ form: 'close-paren' }) nesting-- } while (indents.length) { tokens.push({ form: 'close-paren' }) indents.pop() }
Those could be re-written as for
loops. For example:
for (; nesting > 1; nesting--) {
tokens.push({
form: 'close-paren'
})
}
for (; indents.length; indents.pop()) {
tokens.push({
form: 'close-paren'
})
}
Additionally, the following three lines appear eight times:
tokens.push({ form: 'close-paren' })
Making a separate function/method for that seems excessive (since it would lead to additional function calls just to call the push method on an array). It could be declared once before the function:
const closeParenToken = {
form: 'close-paren'
};
And then referenced whenever such a token needs to be pushed into the tokens. If having the same object copied each time is an issue then various techniques could be used - e.g. spreading into a new object - {...closeParenToken}
Other review points
Most variables are declared using let
. Some could be could be declared with const
- e.g. indents
, match
on line 41, match
on line 62, newIndent
, oldIndent
, etc. It is wise to default to using const, unless you know re-assignment is necessary - then use let. This will help avoid accidental re-assignment and other bugs .
Let's look at the second question first:
Or somehow make that first branch clean?
It appears that you already started to analyze that lex
function. At 137 lines it is a massive function to read through, let alone try updating. Think of anyone reading the code, including your future self. It would be wise to break that function up into smaller functions. This will also help with testing.
One option would be to use OOP - even if only one instance of an object is needed that object could hold the state in properties instead of referencing variables in a large function. Then that monstrous lex
function can be broken up into smaller methods that use properties to update the state.
Is there anything repetitive which could be abstracted out or simplified?
Yes there are quite a few repetitive blocks. For example, the following two blocks appear both within the block starting on line 45 (i.e. if (str.match(/^ *$/)) {
) as well as just before the return statement:
while (nesting > 1) { tokens.push({ form: 'close-paren' }) nesting-- } while (indents.length) { tokens.push({ form: 'close-paren' }) indents.pop() }
Those could be re-written as for
loops. For example:
for (; nesting > 1; nesting--) {
tokens.push({
form: 'close-paren'
})
}
for (; indents.length; indents.pop()) {
tokens.push({
form: 'close-paren'
})
}
Additionally, the following three lines appear eight times:
tokens.push({ form: 'close-paren' })
Making a separate function/method for that seems excessive (since it would lead to additional function calls just to call the push method on an array). It could be declared once before the function:
const closeParenToken = {
form: 'close-paren'
};
And then referenced whenever such a token needs to be pushed into the tokens. If having the same object copied each time is an issue then various techniques could be used - e.g. spreading into a new object - {...closeParenToken}