patterncsharpMinor
Of Procedures and Variables: never enough nodes
Viewed 0 times
proceduresnevernodesvariablesandenough
Problem
Building on my ANTLR tree listener, I'm now starting to see how the whole thing is coming together.
As I proceed to implement the numerous
ProcedureNode
Used for
```
public class ProcedureNode : Node
{
public enum VBProcedureKind
{
Sub,
Function,
PropertyGet,
PropertyLet,
PropertySet
}
public ProcedureNode(VisualBasic6Parser.PropertySetStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.PropertySet, context.visibility(), context.ambiguousIdentifier(), null)
{
}
public ProcedureNode(VisualBasic6Parser.PropertyLetStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.PropertyLet, context.visibility(), context.ambiguousIdentifier(), null)
{
}
public ProcedureNode(VisualBasic6Parser.PropertyGetStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.PropertyGet, context.visibility(), context.ambiguousIdentifier(), context.asTypeClause)
{
}
public ProcedureNode(VisualBasic6Parser.FunctionStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.Function, context.visibility(), context.ambiguousIdentifier(), context.asTypeClause)
{
}
public ProcedureNode(VisualBasic6Parser.SubStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.Sub, context.visibility(), context.ambiguousIdentifier(), null)
{
}
private ProcedureNode(ParserRuleContext context, stri
As I proceed to implement the numerous
Node classes I'm going to need to make sense out of the ANTLR parse tree, I feel like I'm doing more and more weird things.. oh, the tests pass. But the implementations... here, have a look:ProcedureNode
Used for
Sub, Function, Property Get, Property Let and Property Set code blocks, this node can have children and defines a scope.```
public class ProcedureNode : Node
{
public enum VBProcedureKind
{
Sub,
Function,
PropertyGet,
PropertyLet,
PropertySet
}
public ProcedureNode(VisualBasic6Parser.PropertySetStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.PropertySet, context.visibility(), context.ambiguousIdentifier(), null)
{
}
public ProcedureNode(VisualBasic6Parser.PropertyLetStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.PropertyLet, context.visibility(), context.ambiguousIdentifier(), null)
{
}
public ProcedureNode(VisualBasic6Parser.PropertyGetStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.PropertyGet, context.visibility(), context.ambiguousIdentifier(), context.asTypeClause)
{
}
public ProcedureNode(VisualBasic6Parser.FunctionStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.Function, context.visibility(), context.ambiguousIdentifier(), context.asTypeClause)
{
}
public ProcedureNode(VisualBasic6Parser.SubStmtContext context, string scope, string localScope)
: this(context, scope, localScope, VBProcedureKind.Sub, context.visibility(), context.ambiguousIdentifier(), null)
{
}
private ProcedureNode(ParserRuleContext context, stri
Solution
ProcedureNode constructor
The private constructor can be simplified / made be more readable by returning early if
You could consider to change the
ParserRuleContextExtensions
With the given name of the class, the
VariableNode constructor
After reordering the assignments in the way
Storing the result of
The private constructor can be simplified / made be more readable by returning early if
asType == null. You also shouldn't check for returnTypeClause == null twice. You could consider to change the
private readonly string _returnType; to a "normal" string and add a private setter to the property. By doing this you could get rid of the tenary expression by setting _returnType = ReservedKeywords.Variant at initialization.private ProcedureNode(ParserRuleContext context, string scope, string localScope,
VBProcedureKind kind,
VisualBasic6Parser.VisibilityContext visibility,
VisualBasic6Parser.AmbiguousIdentifierContext name,
Func asType)
: base(context, scope, localScope)
{
_kind = kind;
_name = name.GetText();
_accessibility = visibility.GetAccessibility();
if (asType == null) { return; }
var returnTypeClause = asType();
var returnTypeClauseIsNull = returnTypeClause == null;
_isImplicitReturnType = returnTypeClauseIsNull;
_returnType = returnTypeClauseIsNull
? ReservedKeywords.Variant
: returnTypeClause.type().GetText();
}ParserRuleContextExtensions
With the given name of the class, the
GetAccessibility() method doesn't belong there. If you consider to keep both methods in this class, you should consider to rename the inputparameters to parserRuleContext and visibilityContext. VariableNode constructor
After reordering the assignments in the way
_accessibility is assigned at the top you can reduce horizontal spacing by using guard clauses. Storing the result of
context.asTypeClause() and context.typeHint() will reduce code duplication.public VariableNode(VisualBasic6Parser.VariableSubStmtContext context, string scope,
VisualBasic6Parser.VisibilityContext visibility, bool isLocal = true)
: base(context, scope)
{
_name = context.ambiguousIdentifier().GetText();
_accessibility = isLocal ? VBAccessibility.Private : visibility.GetAccessibility();
var asTypeClause = context.asTypeClause();
if (asTypeClause != null)
{
_typeName = asTypeClause.type().GetText();
return;
}
var hint = context.typeHint();
if (hint == null)
{
_isImplicitlyTyped = true;
_typeName = ReservedKeywords.Variant;
return;
}
_isUsingTypeHint = true;
_typeName = TypeSpecifiers[hint.GetText()];
}Code Snippets
private ProcedureNode(ParserRuleContext context, string scope, string localScope,
VBProcedureKind kind,
VisualBasic6Parser.VisibilityContext visibility,
VisualBasic6Parser.AmbiguousIdentifierContext name,
Func<VisualBasic6Parser.AsTypeClauseContext> asType)
: base(context, scope, localScope)
{
_kind = kind;
_name = name.GetText();
_accessibility = visibility.GetAccessibility();
if (asType == null) { return; }
var returnTypeClause = asType();
var returnTypeClauseIsNull = returnTypeClause == null;
_isImplicitReturnType = returnTypeClauseIsNull;
_returnType = returnTypeClauseIsNull
? ReservedKeywords.Variant
: returnTypeClause.type().GetText();
}public VariableNode(VisualBasic6Parser.VariableSubStmtContext context, string scope,
VisualBasic6Parser.VisibilityContext visibility, bool isLocal = true)
: base(context, scope)
{
_name = context.ambiguousIdentifier().GetText();
_accessibility = isLocal ? VBAccessibility.Private : visibility.GetAccessibility();
var asTypeClause = context.asTypeClause();
if (asTypeClause != null)
{
_typeName = asTypeClause.type().GetText();
return;
}
var hint = context.typeHint();
if (hint == null)
{
_isImplicitlyTyped = true;
_typeName = ReservedKeywords.Variant;
return;
}
_isUsingTypeHint = true;
_typeName = TypeSpecifiers[hint.GetText()];
}Context
StackExchange Code Review Q#78583, answer score: 5
Revisions (0)
No revisions yet.