-
Notifications
You must be signed in to change notification settings - Fork 37
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
JAAOUAN TAHA #7
base: develop
Are you sure you want to change the base?
JAAOUAN TAHA #7
Conversation
import java.util.Hashtable; | ||
|
||
/** | ||
* AnonymousRule is a Java class that defines a rule for linting of Java code. |
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.
+1 for the good javadoc
|
||
Hashtable<String,Integer> anonymousInstanciations = new Hashtable<>(); | ||
compilationUnit.accept(new AnonymousVisitor(anonymousInstanciations), null); | ||
if(anonymousInstanciations.size()!=0){ |
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.
use !isEmpty
|
||
|
||
|
||
public static void jsonOutputFile(Collection<Violation> violations, Writer writer) throws IOException { |
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.
use a library to wrie json, like fasterxml or Gson
writer.write(json.toString()); | ||
writer.close(); | ||
} catch (IOException e) { | ||
e.printStackTrace(); |
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.
e.printStackTrace
is nit enough to decide the execution flow
@Override | ||
public void visit(IfStmt n, List<RuleWrapper> arg) { | ||
if (!n.getThenStmt().isBlockStmt()) { | ||
missingIfBrackets.add(new RuleWrapper(n.toString() ,n.getRange().isPresent()?n.getRange().get().begin.line:0)); |
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.
avoid ternary operations
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.
why?
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 , because here you can use Optional
chaining.
2nd readability of code is hard when using ternary operations
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.
it's ok, thank u
List<VariableDeclarator> localVariables = n.findAll(VariableDeclarator.class); | ||
for (VariableDeclarator localVariable : localVariables) { | ||
String locVar= localVariable.toString().split("=")[0] ; | ||
if (!Character.isLowerCase(locVar.charAt(0))) { |
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.
!Character.isLowerCase
==> Character.isUpperCase
} | ||
@Override | ||
public void visit(MethodDeclaration n, List<RuleCountWrapper> arg) { | ||
super.visit(n, arg); |
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.
the Visitor
pattern requires that the call to superior visits to be done at the end
public void visit(MethodDeclaration n, List<RuleCountWrapper> arg) { | ||
super.visit(n, arg); | ||
int parameterCount = n.getParameters().size(); | ||
int line = n.getRange().isPresent()?n.getRange().get().begin.line:0; |
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.
use Optional
chaining
int line = n.getRange()
.map(r -> r.begin)
.map(b -> b.line)
.orElse(0);
No description provided.