Métriques subjectifs !
Best Practices
"Code smell, also known as bad smell, in computer programming code, refers to any symptom in the source code of a program that possibly indicates a deeper problem. According to Martin Fowler, "a code smell is a surface indication that usually corresponds to a deeper problem in the system"
Wikipedia
public String validate(final OrderPivot input) {
StringBuilder outputErrorBuilder = new StringBuilder();
// validation du root
if (StringUtils.isBlank(input.getOrder())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "Order").toString());
}
if (StringUtils.isBlank(input.getLastEvent())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "LastEvent").toString());
}
if (input.getLastEventDate() == null) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "LastEventDate").toString());
}
if (StringUtils.isBlank(input.getOrderType())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "OrderType").toString());
}
if (StringUtils.isBlank(input.getSortingSiteId())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "SortingSiteId").toString());
}
if (StringUtils.isBlank(input.getTurnoverStoreId())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "TurnoverStoreId").toString());
}
if (StringUtils.isBlank(input.getLoyaltyCard())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "LoyaltyCard").toString());
}
// validation du Shipment
if (isValidateShipment(input.getShipment())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "Shipment").toString());
}
// validation des Lines
//...next on the next page
}
Null Cheks
if(input.getLines().isEmpty() ) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "Lines").toString());
}else {
List<SkuLine> lineInError = input.getLines().stream().filter(TranfertRequestValidator::isValidateSkuLine)
.collect(Collectors.toList());
if ( !lineInError.isEmpty()) {
outputErrorBuilder.append("[ Lines fields must be not empty for lineNumber ");
// doit etre fais apres si un des lineNumber est null
List<Integer> lineNumberInError = lineInError.stream().map(SkuLine::getLineNumber)
.collect(Collectors.toList());
for (Integer nb : lineNumberInError) {
outputErrorBuilder.append('{').append(nb).append('}');
}
outputErrorBuilder.append(']');
}
}
// validation du shipmentManifest
if(validateShipmentManifest(input.getShipmentManifest())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "ShipmentManifest").toString());
}
String outPutError = outputErrorBuilder.toString();
if(!outPutError.isEmpty()) {
return outPutError;
}else {
return null;
}Redondant Stream usage + for loop !
public String validate(final OrderPivot input) {
StringBuilder outputErrorBuilder = new StringBuilder();
// validation du root
validateStringField(input.getOrder(), outputErrorBuilder, "Order");
validateStringField(input.getLastEvent(), outputErrorBuilder, "LastEvent");
validateNotNullField(input.getLastEventDate(), outputErrorBuilder, "LastEventDate");
validateStringField(input.getOrderType(), outputErrorBuilder, "OrderType");
validateStringField(input.getSortingSiteId(), outputErrorBuilder, "SortingSiteId");
validateStringField(input.getTurnoverStoreId(), outputErrorBuilder, "TurnoverStoreId");
validateStringField(input.getLoyaltyCard(), outputErrorBuilder, "LoyaltyCard");
// validation du Shipment
// on next page
}
private void validateNotNullField(final Object input, final StringBuilder outputErrorBuilder,
final String fieldName) {
if (input == null) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", fieldName).toString());
}
}
private void validateStringField(final String input, final StringBuilder outputErrorBuilder,
final String fieldName) {
if (StringUtils.isBlank(input)) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", fieldName).toString());
}
} // validation des Lines
if (input.getLines().isEmpty()) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "Lines").toString());
} else {
String lineInError = input.getLines().stream().filter(TranfertRequestValidator::isValidateSkuLine)
.map(SkuLine::getLineNumber).map(ln -> ln == null ? LINE_NUMBER_EMPTY : ln.toString())
.collect(Collectors.joining(DELIMITER_DOUBLE, DELIMITER_PREFIX, DELIMITER_SUFFIX));
if (!DELIMITER_SUF_PRE.equals(lineInError)) {
outputErrorBuilder.append("[ Lines fields must be not empty for lineNumber ");
outputErrorBuilder.append(lineInError);
outputErrorBuilder.append(']');
}
}
// validation du shipmentManifest
if (validateShipmentManifest(input.getShipmentManifest())) {
outputErrorBuilder.append(new KiabiMessage("error.field.is.empty", "ShipmentManifest").toString());
}
String outPutError = outputErrorBuilder.toString();
if (!outPutError.isEmpty()) {
return outPutError;
} else {
return null;
}private Boolean validateShipmentManifest(final ShipmentManifest inputShipmentManifest) {
if(inputShipmentManifest == null) {
return Boolean.TRUE;
}
if (inputShipmentManifest.getShipmentNumber()== null) {
return Boolean.TRUE;
}
if (StringUtils.isBlank(inputShipmentManifest.getHubCode())) {
return Boolean.TRUE;
}
if (StringUtils.isBlank(inputShipmentManifest.getParcelNumber())) {
return Boolean.TRUE;
}
if (StringUtils.isBlank(inputShipmentManifest.getParcelNumberExtended())) {
return Boolean.TRUE;
}
if (StringUtils.isBlank(inputShipmentManifest.getInternalParcelNumber())) {
return Boolean.TRUE;
}
if (inputShipmentManifest.getWeight() == null) {
return Boolean.TRUE;
}
if (StringUtils.isBlank(inputShipmentManifest.getHubParcelNumber())) {
return Boolean.TRUE;
}
if (StringUtils.isBlank(inputShipmentManifest.getCartonType())) {
return Boolean.TRUE;
}
if (inputShipmentManifest.getTransfertNumber() == null) {
return Boolean.TRUE;
}
if (inputShipmentManifest.getShipmentDate() == null) {
return Boolean.TRUE;
}
if (inputShipmentManifest.getPackingDate() ==null) {
return Boolean.TRUE;
}
return Boolean.FALSE;
}Too Many If
private Boolean validateShipmentManifest(final ShipmentManifest inputShipmentManifest) {
return inputShipmentManifest == null || inputShipmentManifest.getShipmentNumber() == null
|| StringUtils.isBlank(inputShipmentManifest.getHubCode())
|| StringUtils.isBlank(inputShipmentManifest.getParcelNumber())
|| StringUtils.isBlank(inputShipmentManifest.getParcelNumberExtended())
|| StringUtils.isBlank(inputShipmentManifest.getInternalParcelNumber())
|| inputShipmentManifest.getWeight() == null
|| StringUtils.isBlank(inputShipmentManifest.getHubParcelNumber())
|| StringUtils.isBlank(inputShipmentManifest.getCartonType())
|| inputShipmentManifest.getTransfertNumber() == null || inputShipmentManifest.getShipmentDate() == null
|| inputShipmentManifest.getPackingDate() == null;
}public void format(final LogEvent event, final StringBuilder toAppendTo) {
final Message msg = event.getMessage();
//KIA : START Kiabi customization
Throwable throwable = event.getThrown();
String exceptionCode = KiabiException.UNKNOWN_EXCEPTION_CODE;
if(throwable != null){
if(throwable instanceof KiabiException){
//if it's a KiabiException : we have an exception cause
exceptionCode = ((KiabiException) throwable).getExceptionCode();
}
else {
//else, we check on known exception types on the exception itself or it's causes
Throwable toCheck = throwable;
while(toCheck != null && exceptionCode.equals(KiabiException.UNKNOWN_EXCEPTION_CODE)){
if(toCheck instanceof SQLException){
exceptionCode = ExceptionCodeExtractor.fromSqlException((SQLException) toCheck);
}
else if(toCheck instanceof NamingException){
exceptionCode = ExceptionCodeExtractor.fromNamingException((NamingException) toCheck);
}
else if(toCheck instanceof KiabiException){
//sometimes a KiabiException can be the cause of an other exception, so we need to check this also
exceptionCode = ((KiabiException) throwable).getExceptionCode();
}
//check on the cause
toCheck = toCheck.getCause();
}
}
//TODO make clever analyse on exception to add exception code whenever it's possible
toAppendTo.append(exceptionCode).append(" - ");
}
if (msg instanceof StringBuilderFormattable) {
final int offset = toAppendTo.length();
((StringBuilderFormattable) msg).formatTo(toAppendTo);
//... format of the message
}Deep Nested Ifs
Single Reponsibility Principle !
public void format(final LogEvent event, final StringBuilder toAppendTo) {
final Message msg = event.getMessage();
//KIA : START Kiabi customization
if(event.getThrown() != null) {
//add exception code
String exceptionCode = ExceptionCodeExtractor.extractExceptionCode(event.getThrown());
toAppendTo.append(exceptionCode).append(" - ");
}
//KIA : STOP Kiabi customization
//WARNING : bellow this line, this is taken from org.apache.logging.log4j.core.pattern.MessagePatternConverter
//be carefull that upgrading log4j2 could lead to modification needed on this code and that needs to be manually done!
if (msg instanceof StringBuilderFormattable) {
final int offset = toAppendTo.length();
((StringBuilderFormattable) msg).formatTo(toAppendTo);
//... format the messaye
}
public static String extractExceptionCode(Throwable throwable) {
//if the exception null (or if we go to all the causes and didn't find a match) : return UNKNOWN_EXCEPTION_CODE
if(throwable == null) {
return KiabiException.UNKNOWN_EXCEPTION_CODE;
}
//check on known exception types to extract a code
if(throwable instanceof KiabiException){
return ((KiabiException) throwable).getExceptionCode();
}
else if(throwable instanceof SQLException){
return fromSqlException((SQLException) throwable);
}
else if(throwable instanceof NamingException){
return fromNamingException((NamingException) throwable);
}
//recursive check on the cause
return extractExceptionCode(throwable.getCause());
}