Archive

Articles taggués ‘refactoring’

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 :-P

Ê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…

Share

Dé-switcher n’est pas jouer

Amis utilisateurs de Checkstyle, avez-vous remarqué que les switch sont des sources inépuisables de complexité cyclomatique ? Sans parler de leur équivalent pour les objets, les if () {} else if (){} else if (){} else if (){} à répétition.

Parmi les reproches récurrents faits aux switch, il y a le fait qu’ils ne respectent pas le principe Ouvert/Fermé. A chaque ajout de nouvelles valeurs, il faut modifier tous les switch qui les manipulent.

Lire la suite…

Share