-
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
Khattala Mohammed #6
base: develop
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,18 @@ | |||
package com.ensao.gi5.test.constantes; | |||
|
|||
public class Paths { |
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 refactoring paths
@@ -12,55 +12,54 @@ public class ImportWrapper { | |||
private final String importDeclarationFQN; |
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 file contains unnecessary changes, be aware of the indentations :)
|
||
@Override | ||
public void visit(ClassOrInterfaceDeclaration n, Set<ElementWrapper> arg) { | ||
arg.add(new ElementWrapper(n.getNameAsString(), n.getBegin().map(p -> p.line).orElse(-1))); |
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.
Chained method calls are preferable to be separated by a new line, like :
arg.add(new ElementWrapper(n.getNameAsString(), n.getBegin()
.map(p -> p.line)
.orElse(-1)));
|
||
@Override | ||
public void visit(MethodDeclaration n, Set<ElementWrapper> localVariableWrappers) { | ||
final BlockStmt body = n.getBody().get(); |
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.
Optional::get
is not safe if the Optional
instance is empty.
use Optional::isPresent
before accessing the inner element.
writer.close(); | ||
} | ||
|
||
public static void writeJson(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.
could be
public static void writeJson(Collection<Violation> violations, Writer writer) throws IOException {
Gson gson = new Gson();
String obj = gson.toJson(violations);
writer.append(obj);
writer.flush();
writer.close();
}
Gson
can convert a Collection
of objects into a JsonArray
import java.util.Collection; | ||
|
||
public class Utils { | ||
Utils() { |
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.
be aware of indentations
public void apply(CompilationUnitWrapper compilationUnit) { | ||
final Set<ElementWrapper> names = new LinkedHashSet<>(); | ||
compilationUnit.accept(new TypeNameVisitor(), names); | ||
names.stream().filter(n->!Character.isUpperCase(n.getName().charAt(0))).forEach(n->{ |
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.isUpperCase
==> Character.isLowerCase
Set<MethodDeclaration> mds = new HashSet<MethodDeclaration>(); | ||
compilationUnit.accept(new MethodVisitor(), mds); | ||
mds.stream() | ||
.filter(m -> m.getEnd().map(p -> p.line).orElse(-1) - m.getBegin().map(p -> p.line).orElse(-1) + 1 > 30) |
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.
for more readability
....
.filter(m -> {
final int endLine = m.getEnd()
.map(p -> p.line)
.orElse(-1);
final int beginLine = m.getBegin()
.map(p -> p.line)
.orElse(-1);
return endLine - beginLine + 1 > 30;
})
....
|
||
@Override | ||
public void apply(CompilationUnitWrapper compilationUnit) { | ||
Set<BinaryExpr> bExps = new LinkedHashSet<BinaryExpr>(); |
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.
could be Set<BinaryExpr> bExps = new LinkedHashSet<>();
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 appreciate your relevant remarks sir and i promess i will take them into consideration.
No description provided.