New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor parsePrefixExpression, block some expressions #189 #191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thank you for contributing to our project! @u5surf 👍
Feedback
The logic is ok. But I think we can refactor and improve more.
- How about validating operation and right expression first, then create whole expression?
The old code create whole expression first then filling the operation and right expression. I think after validating op and right expression then creates whole expression is more make sense.
func parsePrefixExpression(buf TokenBuffer) (ast.Expression, error) {
tok := buf.Read()
op := operatorMap[tok.Type]
right, err := parseExpression(buf, PREFIX)
// validating logic
// create whole expression
}
- We can use
switch
instead ofif
. I think usingswitch
can be more readable and scalable.
switch op {
case ast.Bang:
...
case ast.Minus:
...
}
and
switch right.(type) {
case *ast.BooleanLiteral:
...
case *ast.StringLiteral:
...
}
Error has occurred !!
plz fix the error |
There is still an error in the code. |
Oh, I got it, parseError has added some element imperceptibly! |
@@ -367,16 +367,38 @@ func parseInfixExpression(buf TokenBuffer, left ast.Expression) (ast.Expression, | |||
func parsePrefixExpression(buf TokenBuffer) (ast.Expression, error) { | |||
var err error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this err
is not needed
resolved: #189
details: I added validation rules to block prefix ‘!‘ is not valid for StringLiteral.
and ‘-’ is also invalid for Boolean.
[task detail...]
I fix TestMakePrefix to pass invalid error meesage to indicate blocking following [parser] refactor parsePrefixExpression, block some expressions #189 cases.