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
Code review of the Ast #2941
Code review of the Ast #2941
Conversation
} | ||
|
||
//************************************************************ | ||
|
||
final class Infix(val parts: List[String], val params: List[Ast], val pure: Boolean, val transparent: Boolean)( | ||
theQuat: => Quat | ||
) extends Ast { | ||
def quat: Quat = theQuat |
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.
A few theQuat
calls weren't "cached" and were potentially re-computed many times
} | ||
|
||
final class Ident private (val name: String)(theQuat: => Quat)(val visibility: Visibility) extends Terminal with Ast { | ||
private lazy val computedQuat = theQuat |
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.
We don't need this computedQuat
variable. the overrided def quat: Quat
method can be just a lazy val
@@ -422,7 +425,7 @@ object Visibility extends OpinionValues[Visibility] { | |||
} | |||
|
|||
sealed trait Renameable extends Opinion[Renameable] { | |||
def fixedOr[T](plain: T)(otherwise: T) = | |||
def fixedOr[T](plain: => T)(otherwise: => T): T = |
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.
This wasn't lazy enough. Always computing both sides even if one of them is never used
No description provided.