Skip to content
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

ensao-lint (Mohamed El kadiri) #28

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Gilgamesh1302
Copy link

Hello sir,
please free to share any advice or remarks regarding my work, that would be much appreciated.
Thank you in advance.


public class RuleFactory {
public Rule getRule(RulesEnum ruleSubClass) {
return switch(ruleSubClass) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽

}

public boolean isAttributeNotConstant(AttributeWrapper attribute) {
return !attribute.isStatic() && !attribute.isFinal();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is final, it doesn't mean it is static
example

private final String foo;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to ckeck for constants because they should start with an uppercase thats why i checked for both in this case

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.
a constant is static && final,
the opposite is not (static && final) so the condition will be isAttributeNotConstant => not static || not final

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the clarification

List<AttributeWrapper> attributes = new ArrayList<>();
compilationUnit.accept(new AttributeVisitor(), attributes);
for (AttributeWrapper attribute: attributes) {
if (attribute.getAccessModifier().isEmpty()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is a wrapper, put this logic in wrapped method

if(attribute.hasNoAccessModifiers()) { .....

List<BooleanExpressionWrapper> booleanExpressions = new ArrayList<>();
compilationUnit.accept(new BooleanExpressionVisitor(), booleanExpressions);
for (BooleanExpressionWrapper booleanExpression: booleanExpressions) {
if (checkIfBooleanExpressionHasMoreThanTwoOperands(booleanExpression)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkIfBooleanExpressionHasMoreThanTwoOperands ==> booleanExpressionHasMoreThanTwoOperands
in the reading it will be if boolean expression has more than two operands

}

public boolean checkIfBooleanExpressionHasMoreThanTwoOperands(BooleanExpressionWrapper booleanWrapper) {
String[] booleanOperands = booleanWrapper.getCondition().split("(&&) | (\\|\\|)");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be done by a visitor pattern

List<MethodWrapper> methods = new ArrayList<>();
compilationUnit.accept(new MethodVisitor(), methods);
for (MethodWrapper method: methods) {
if (checkIfMethodHasMoreThanThirtyLines(method)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkIfMethodHasMoreThanThirtyLines ==> methodHasMoreThanThirtyLines

}

public boolean checkIfMethodHasMoreThanThirtyLines(MethodWrapper methodWrapper) {
return methodWrapper.getStatements().size() > 30;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put this logic into the wrapper class

}

public static String getTypeNameFromNodes(Node node) {
return ((TypeDeclaration<?>) node.getParentNode().get()).getNameAsString();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional::get is to be called conditionally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants