Head in the wall

I'm working as a consultant in a big company. Here are some samples of code I can see every day ...

[codeleaks]

isAllowedToWriteCode ...

private boolean isAllowToImportNote(User author) { // if not admin if (!isAdmin) { // if not manager if (!isManager) { // check if the user can create a note. if (importInfoDTO.getCurrentUser().getName().equals(author.getName())) return true; else // Nothing to do return false; } else return true; } else return true; }

Qu'est ce qui est pire …

L'absence d'accolades ?

Sa réponse “ça améliore les perfs” ?

Ses commentaires d'une grande nécessité ?

Les 4 returns ?

Le fait de pouvoir réécrire cette méthode en 3 lignes (accolades comprises !) ?

[codeleaks]

Method name overflow

public void decoratedApplicationContextYieldsADifferentOutputStreamIfArgumentsMapContainsAValueForItAndTheSpecifiedFileCanBeOpened() {

[codeleaks]

What does it do ?

private static String detail(List <Integer> integers, int limite) { int[] i = new int[limite+1]; for (Integer integer : integers) { if(integer < limite) { i[integer] ++; } else { i[limite] ++; } } String result = ""; for (int j = 0; j < limite; j ++ ) { if( i[j] != 0 ) { result += j + " : " + i[j] + "\n"; } } if (i[limite] != 0) { result += ">" + limite + " : " + i[limite] + "\n"; } return result; }

[codeleaks]

8 arguments ... enough ?

memberFunctionName = "call" + parametersArray.size(); if (expectedParameterCount > 0) { method.invoke(SDCFunction, context, Arrays.copyOfRange(parametersArray, 0, expectedParameterCount)); } else { method.invoke(SDCFunction, context); } // LATER IN THE CLASS .... public Object call0(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context)); } catch (Exception e) { throw new RuntimeException(e); } } public Object call1(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0])); } catch (Exception e) { throw new RuntimeException(e); } } public Object call2(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0], args[1])); } catch (Exception e) { throw new RuntimeException(e); } } public Object call3(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0], args[1], args[2])); } catch (Exception e) { throw new RuntimeException(e); } } public Object call4(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0], args[1], args[2], args[3])); } catch (Exception e) { throw new RuntimeException(e); } } public Object call5(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0], args[1], args[2], args[3], args[4])); } catch (Exception e) { throw new RuntimeException(e); } } public Object call6(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0], args[1], args[2], args[3], args[4], args[5])); } catch (Exception e) { throw new RuntimeException(e); } } public Object call7(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0], args[1], args[2], args[3], args[4], args[5], args[6])); } catch (Exception e) { throw new RuntimeException(e); } } public Object call8(EvaluationContext context, Object... args) { try { return convert(method.invoke(SDCFunction, context, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7])); } catch (Exception e) { throw new RuntimeException(e); } }

[codeleaks]

IF If iF if ...

if (line.contains("nan") || line.contains("nAn") || line.contains("nAN") || line.contains("NAN") || line.contains("Nan") || line.contains("NaN") || ......

What about

line.toLowerCase().contains("nan")

[codeleaks]

Party mode

Festival ! Fields are public, no getter/setter and there are Integer.valueOf(string) everywhere in the code cause score/min/max are integers ...

public class Point { public String score; public String min; public String max; } List points ...

[codeleaks]

String comparaison in a unit test

Comparing 2 strings in a unit test:

String expected = ... Assert.assertArrayEquals(expected.getBytes(), actual.getBytes());

What about:

Assert.assertEquals(expected, actual);

[codeleaks]

COMBO :o

String[] split = str.split(","); for (String s : split) { value += s.replaceAll(";", "") + ";"; } return value;

[codeleaks]

public void findOutliersYieldsTheFirstOutliersIndexesFoundByTheInnerTestIfTheirAmountIsGreaterThanOutliersLimit(String id) { // SOME CODE }

Obvious: The method "find*" return a void The method write in a DB ... :o

[codeleaks]

A TreeMap dude ?

Create an HashTable with timestamps as key, call a method to sort them and then recreate a map ...

TsMap tsMap = new TsMap(); tsMap.sortPointsByIncreasingTimestamp(); public class TsMap { Map points; public TsMap() { points = new Hashtable() } public boolean sortPointsByIncreasingTimestamp() { ArrayList listTimestamp = new ArrayList(points.keySet()); if (listTimestamp.isEmpty()) return false; Map sortedPoints = new Hashtable(listTimestamp.size()); Collections.sort(listTimestamp); for (Long ts : listTimestamp) sortedPoints.put(ts, points.get(ts)); points = sortedPoints; return true; } public Map() { return points; } }

What about the TreeMap dude ?

[codeleaks]

Back to work after 2 days off

A ast-core/ast-core.iml ! + C ast-core/src/test/java/com/profiling > local edit, incoming delete upon update ? ast-core/src/test/java/com/ast/core/manager/PointDateTest.java M ast-core/src/test/java/com/ast/core/manager/CassSeriesManagerTest.java ! + C ast-core/src/main/java/com/profiling > local edit, incoming delete upon update M ast-core/src/main/java/com/ast/core/data/Point.java A ast-ws/ast-ws.iml A ast-mr/ast-mr.iml A + C ast-mr/src/main/java/com/profiling > local edit, incoming delete upon update C + ast-mr/src/main/java/com/profiling/ast/mr/jobs/reporting/ReportingContextService.java

[codeleaks]

Overriding toString

Next time, create a toCustom() method ...

assertEquals("1;1.0;;;;;", point1.toString()); // :( assertEquals("1;;1.0;1.0;1.0;;", point2.toString()); // :(

[codeleaks]

What the fuck ?

public class DoubleListToArray implements Delegate> { @Override public double[] perform(List list) { dbc.precondition(list != null, "list is null"); final double[] array = new double[list.size()]; int i = 0; for (double value : list) { array[i++] = value; } return array; } }

[codeleaks]

The boolean return pattern

public boolean importFromHDFS(Path location, Configuration conf) { boolean isImported = false; // SOME CODE HERE if (!this.isTrained) { if (type_alg == 1) { isImported = true; // SOME CODE HERE return isImported; } if (type_alg > 1) { if (type_alg == 2) { isImported = true; // SOME CODE HERE return isImported; } if (type_alg == 3) { isImported = true; // SOME CODE HERE return isImported; } } } return isImported; }

[codeleaks]

Complexity

PLEASE, don’t reivent the wheel, use:

ArrayUtils.removeAll(array, indexes)

Instead of:

@Override public double[] perform(double… array, int[] indexes) { dbc.precondition(array != null, “array is null”); if (array.length == 0) { return new double[0]; } final int indexesToUse = containedIndexes(indexes, array.length); final double[] filtered = new double[array.length - indexesToUse]; int index = 0; for (int i = 0; i < filtered.length; i++) { while (index < indexesToUse && (i + index) == indexes[index]) { index++; } filtered[i] = array[i + index]; } return filtered; } private int containedIndexes(int[] indexes, int length) { final int binarySearch = Arrays.binarySearch(indexes, length); return binarySearch < 0 ? ~binarySearch : binarySearch; }

[codeleaks]

Mathematician developper

public static double distanceCal2(double lat1, double lat2, double lon1, double lon2) { //another formula to calculate the distance double R = 6371; // km double φ1 = Math.toRadians(lat1); double φ2 = Math.toRadians(lat2); double Δφ = Math.toRadians(lat2-lat1); double Δλ = Math.toRadians(lon2-lon1); double d = R * Math.acos(Math.cos(φ1)*Math.cos(φ2)*Math.cos(Δλ) + Math.sin(φ1)*Math.sin(φ2)); return d; }

[codeleaks]

Which one to use ?

anObject.equals(Object obj) anObject.almostEquals(Object obj) anObject.almostEquals(Object obj, Double precision)