sevntu-checkstyle / methods-distance

GNU Lesser General Public License v3.0
5 stars 13 forks source link

Code review: phase 2 #5

Closed romani closed 8 years ago

romani commented 8 years ago

1)

package com.github.sevntu.checkstyle.analysis;

please rename "analysis" to "domain" as it will be collection of POJO classes that make a domain structure of dependencies that is build by smth from source file and and could be serialized to any(unknown) destination.

2)

public class AnalysisSubject

public class that contains only static content - make it a Util class. No need to keep it as base class for others. The shorter hierarchy the better.


ClassDefinition.java

3)

isInsideMethodOfClass, ... getClassName ,....

All such method should be calculated in c-tor and just give to outside ready to use content. If it is required occasionally - it should be util method, not a part of this domain class. Such data can not be changed.

4)

return methods.stream()
                .filter(method -> method.getIndex() == index)
                .findFirst()
                .get();

you mean methods.get(index) ? please remove all extra java8 usage it does not show you knowledge from good side.

5)

return getMethodsByName(methodName).stream()

do not reuse getMethodsByName, you does not make you code shorter, but create too much extra objects and logic is too abstract.


6)

DependencyInformationConsumer

move this class out of this package


7)

MethodCall

how such public method could return different results on their calls, please pre-calculate all required information and return ready to use answers. I doubt you need here lazy calculations.


8)

MethodDefinition

this cass is too big, it should by like a structure of ready to use data for Serializers. Looks like you need to have Builder of this class from AST structure.

9)

PenaltyCalculator.java

move this class out of this package

10)

UnexpectedTokenTypeException.java

move this class out of this package