Simplifying Code
In the course of trying to adapt an ANTLR lexer to be used in an Intellij IDEA language plugin, I found that I’d written this:
token = lexer.nextToken();
if (token != null)
{
tokenStart = findCharPos(token.getLine(), token.getColumn());
if (token.getText() == null)
{
tokenEnd = tokenStart;
}
else
{
tokenEnd = tokenStart + token.getText().length() - 1;
state = tokenEnd;
}
if (token.getType() == 1)
{
state = -1;
}
}
else
{
tokenStart = 0;
tokenEnd = 0;
state = -1;
}
What a mess! Which parts of the code influence the final value of tokenEnd? Should state be modified in the token.getText() == null case?
The problem with the code above is that it is structured as cases of the state of token, rather than as the functions which determine the values of the three variables.
I changed it to:
token = lexer.nextToken();
tokenStart = calcTokenStart(token);
tokenEnd = calcTokenEnd(tokenStart, token);
state = calcState(tokenEnd, token);
...
private int calcTokenStart(Token token)
{
return token == null ? 0 : findCharPos(token.getLine(), token.getColumn());
}
private int calcTokenEnd(int tokenStart, Token token)
{
if (token == null)
{
return 0;
}
else
{
String tokenText = token.getText();
return tokenText == null ? tokenStart : tokenStart + tokenText.length();
}
}
private int calcState(int tokenEnd, Token token)
{
if (token == null || token.getType() == 1)
{
return -1;
}
else
{
return tokenEnd;
}
}
This shows us each calculation in isolation, and allows us to see its dependencies.Note that this isn’t an ‘extract method’ refactoring — it comes from changing your view of the function from ‘processing a new token’ to ‘calculating the new values of the three state variables’.
Note that the functions above don’t use the state of the object, even though, for instance, this.tokenStart has been assigned when calcTokenEnd is called. That would reduce the clarity of the solution.
In fact, unless this class was specifically designed to be extensible, perhaps making these functions static would be clearer and safer.
