Refactoring par la pratique
Dans cet article, nous allons procéder à un refactoring sur un exemple concret, afin de mettre en exergue quelques bonnes pratiques.
Défi : comprendre du code inélégant*
*Edit : j’avais initialement écrit “imb*table”, mais il paraît que ça ne fait pas très sérieux
Êtes-vous capable de comprendre ce que fait la méthode computeConvexHull()
en moins de 3 minutes ?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86 public List<GeoPoint> computeConvexHull(Set<GeoPoint> points) {
// Path list
ArrayList<GeoPoint> path = new ArrayList<GeoPoint>();
// Pick the min longitude point as first
GeoPoint first = null;
for (GeoPoint point : points) {
if (first == null || point.getLongitude() < first.getLongitude()) {
first = point;
}
}
if (first != null) {
path.add(first);
} else {
throw new IllegalArgumentException("Can't retrieve the western point");
}
boolean right = true;
boolean loop = true;
while (loop) {
GeoPoint current = path.get(path.size() - 1);
GeoPoint next = null;
double nextLatitudeDiff = 0;
double nextLongitudeDiff = 0;
boolean recordNext = false;
for (GeoPoint point : points) {
if (!point.equals(current)) {
double latitudeDiff = point.getLatitude() - current.getLatitude();
double longitudeDiff = point.getLongitude() - current.getLongitude();
if (longitudeDiff == 0 || longitudeDiff > 0 == right) {
if (next == null) {
recordNext = true;
} else {
// Compare the 'a' in the linear equation Y = a.X
// that correspond to the line slope.
boolean negativeSlope = latitudeDiff * nextLongitudeDiff - nextLatitudeDiff * longitudeDiff < 0;
if (negativeSlope) {
recordNext = true;
}
}
if (recordNext) {
recordNext = false;
next = point;
nextLatitudeDiff = latitudeDiff;
nextLongitudeDiff = longitudeDiff;
}
}
}
}
if (next != null) {
// Check abnormal case
if (path.size() > points.size() + 1) {
throw new RuntimeException("Convex hull computation failed");
}
path.add(next);
/*
* If we are not back to the first point, let's continue finding
* the path.
*
* Otherwise, do nothing => going back from the stack, this is
* actually the end point of the recursive algorithm.
*/
if (path.get(0) == next) {
loop = false;
}
} else {
// We have reached the point on the right, let's go back!
right = !right;
}
}
return path;
}
Alors ? Checkstyle lui donne une complexité cyclomatique de 16. Plutôt difficile d’y voir clair. Moi en tout cas, je n’en suis pas capable.
Et pourtant, j’en suis l’auteur :-). Critique de code bien ordonnée commence par soi-même ! Lire la suite…