-
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
MbarkiPr #13
base: develop
Are you sure you want to change the base?
MbarkiPr #13
Conversation
@@ -48,6 +48,24 @@ | |||
<artifactId>commons-io</artifactId> | |||
<version>2.11.0</version> | |||
</dependency> | |||
<!-- https://mvnrepository.com/artifact/com.google.guava/guava --> | |||
<dependency> |
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.
Guava library is not necessary, use java collection framework instead
@@ -46,6 +46,7 @@ public void run() { | |||
final List<CompilationUnitWrapper> compilationUnitWrappers = new ArrayList<>(); | |||
for (File source : sources) { | |||
compilationUnitWrappers.add(Utils.getCompilationUnit(source)); | |||
System.out.println(Utils.getCompilationUnit(source)); |
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.
to be removed
import java.nio.file.Paths; | ||
import java.util.Collection; | ||
|
||
public class CSVFileWriter implements Printer { |
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.
👍🏽
import java.nio.file.Paths; | ||
import java.util.Collection; | ||
|
||
public class HtmlPrinter implements Printer { |
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.
👍🏽
public class JsonPrinter implements Printer{ | ||
@Override | ||
public void printViolations(Collection<Violation> violations) { | ||
JSONArray jsonArray = new JSONArray(); |
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 fasterxml
or Gson
to automatically render json objects
import java.nio.file.Paths; | ||
import java.util.Collection; | ||
|
||
public class MarkdownPrinter implements Printer{ |
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.
👍🏽
compilationUnit.accept(new ConstVisitors(), m); | ||
|
||
|
||
for(String s:m) { |
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 significant variable names, s
and m
are hard to understand
|
||
|
||
|
||
Predicate<Object> p1= p->(Character.isUpperCase(p.toString().charAt(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.
use significant variable names instead of p1
.collect(Collectors.toSet()); | ||
|
||
|
||
if (!answer.isEmpty()) { |
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 condition is useled, the for
loop deals perfectly with empty collections
final Set<NominationWrapper> nominations = new LinkedHashSet<>(); | ||
compilationUnit.accept(new NominationVisitors(), nominations); | ||
for (NominationWrapper m:nominations){System.out.println(m);} | ||
Predicate<Object> p1= p->(Character.isLowerCase(p.toString().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.
Use the correct type instead of Object
No description provided.