Découper une grosse classe

Bonjour à tous,

Je me tourne vers vous parce que j'ai un problème d'architecture auquel je ne parviens pas à trouver une solution. D'ailleurs, le problème est un peu complexe à décrire, je vais tenter d'être le plus clair possible.

Dans une bibliothèque, j'ai une classe abstraite que je dois étendre. Cette classe est nommée ASTVisitor, dispose pas moins de 1025 lignes de code et tout son contenu ne fait que déclarer des méthodes. Ces méthodes sont non abstraites mais je dois en étendre la quasi totalité. Comme vous l'avez peut-être deviné avec son nom, cette classe déclare toutes les méthodes pour visiter un noeud et pour sortir de ce noeud. La classe se représente donc sous la forme suivante :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10

public abstract class ASTVisitor {
    public void endVisit(AllocationExpression allocationExpression, BlockScope scope) {
    }

    public boolean visit(AllocationExpression allocationExpression, BlockScope scope) {
        return true;
    }

    // Beaucoup d'autres méthodes du même genre pour chaque noeud.
}

Pour chaque noeud de la bibliothèque, il existe une méthode traverse qui va, comme son nom l'indique, traverser tous les attributs traversable du noeud courant. Un exemple typique de méthode traverse est le suivant :

 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

public class TryStatement {
    public void traverse(ASTVisitor visitor, BlockScope blockScope) {
        if(visitor.visit(this, blockScope)) {
            LocalDeclaration[] localDeclarations = this.resources;
            int i = 0;

            int max;
            for(max = localDeclarations.length; i < max; ++i) {
                localDeclarations[i].traverse(visitor, this.scope);
            }

            this.tryBlock.traverse(visitor, this.scope);
            if(this.catchArguments != null) {
                i = 0;

                for(max = this.catchBlocks.length; i < max; ++i) {
                    this.catchArguments[i].traverse(visitor, this.scope);
                    this.catchBlocks[i].traverse(visitor, this.scope);
                }
            }

            if(this.finallyBlock != null) {
                this.finallyBlock.traverse(visitor, this.scope);
            }
        }

        visitor.endVisit(this, blockScope);
    }
}

Comme vous pouvez le remarquer, une méthode traverse prend en paramètre un ASTVisitor et un scope. Dans la mesure où le contexte à de l'importance et que je n'ai pas la main sur ces méthodes, cela semble m'obliger à conserver une seule et unique classe qui regroupe l'implémentation voulue pour toutes les méthodes de la classe ASTVisitor. Le problème se situe sur ce point précis.

Aujourd'hui, ma classe qui étend ASTVisitor fait pas moins de 2666 lignes. J'aimerais donc pouvoir découper ma classe pour regrouper les méthodes "similaires" dans des classes distinctes. Par exemple, regrouper toutes les méthodes d'éléments structurants (comme les classes, les méthodes, les champs, etc.). J'ai beau réfléchir au problème, je n'ai pas la moindre idée de comment je pourrais m'y prendre.

Je me tourne donc vers vous pour savoir si vous avez des idées à me proposer. N'hésitez pas, tout est bon à prendre pour m'aider à faire avancer mes réflexions.

Merci d'avance pour votre aide.

Salut !

Peux-tu expliquer ce point à quelqu'un qui ne connait pas trop les spécificités de Java ? Quel est le problème ici ?

Si nous reprenons l'exemple du traverse du try/catch donné dans mon premier message, l'implémentation de ma classe est la suivante :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21

public class JDTTreeBuilder extends ASTVisitor {
    @Override
    public boolean visit(TryStatement tryStatement, BlockScope scope) {
        CtTry t;
        if (tryStatement.resources.length > 0) {
            t = factory.Core().createTryWithResource();
        } else {
            t = factory.Core().createTry();
        }
        context.enter(t, tryStatement);
        return true;
    }

    @Override
    public boolean visit(Argument argument, BlockScope scope) {
        if (this.getContextBuilder().stack.peekFirst().element instanceof CtTry) {
            context.enter(factory.Core().createCatch(), argument);
            return true;
        }
    }
}

Dans cet exemple, je surcharge les méthodes qui visite le try et le catch, je place les objets dans un contexte et je m'en sers pour savoir dans quel cas je suis. Par exemple, la méthode du catch est générique à d'autres cas comme des paramètres de méthode. Puis, je renvoie true pour signaler à ma bibliothèque que je veux continuer à traverser le noeud courant (cf. l'implémentation du traverse donné dans mon premier message qui gère le retour de la méthode visit).

J'espère que je suis un peu plus clair.

Ok. Du coup qu'est-ce qui t'empêche de faire ça (conceptuellement) :

1
2
3
4
5
6
7
8

@Override
public boolean visit(Argument argument, BlockScope scope) {
-    if (this.getContextBuilder().stack.peekFirst().element instanceof CtTry) {
-        context.enter(factory.Core().createCatch(), argument);
-        return true;
-    }
+    return blah(context, argument, scope);
}

As-tu la main tu ta classe abstraite ? Est-ce que tu peux modifier la signature de la méthode traverse par exemple ? Parce que pour découper ça, il me semble que les interfaces devrait pouvoir te permettre de rassembler certaines méthodes.

L'idée étant que ta méthode traverse soit signée ainsi public void traverse(Traversable visitor, BlockScope blockScope) dont Traversable est une interface qui regroupera tes méthodes liées au parcours d'un nœud. ça ne sera pas fait sans un peu structuration ailleurs.

+1 Pour firm1

Il est clair que dès pour où l'on crée des méthode dans une classe abstraite, dans le seul but de les étendre par héritage, il faut utiliser une interface.

Et quand on a une signature similaire, avec seul le type de l'objet qui change dont on a besoin de l'implémentation, il faut penser aux templates.

Si la classe ASTVisitor se contente de déclarer des méthode visit et endvisit avec comme seules différences les paramètres, il faut en faire une interface template.

1
2
3
4
5
6
7

public interface Visitor<T> {

    public boolean visit(T resource, BlockScope scope);

    public void endVisit(T resource, BlockScope scope);

}

Exemple d'implémentation :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17

public class JDTTreeBuilder implements Visitor<TryStatement>
{
    public boolean visit(TryStatement tryStatement, BlockScope scope);

    public void endVisit(TryStatement tryStatement, BlockScope scope);
}

public class ArgumentVisitor implements Visitor<Argument>
{
    public boolean visit(Argument argument, BlockScope scope) {
        ...
    }

    public void endVisit(Argument argument, BlockScope scope) {
        ...
    }
}

En procédant ainsi, tu va pouvoir répartir le millier de ligne de code dans des classes plus petites.

Pour aller plus loin, on peut créer une classe abstraite avec du code générique pour éviter les copier coller de code similaire.

Voici le code qui montre le principe. Toutefois, ta hiérarchie des classes que tu nous montre n'est pas complète. Aussi, l'exemple ci après ne répond pas exactement à ta problématique.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12

public abstract class ATraverser {

    protected abstract void doTraverse(Visitor<ATraverser> visitor, BlockScope blockScope);

    public void traverse(Visitor<ATraverser> visitor, BlockScope blockScope) {
    if( visitor.visit(this, blockScope) ) {
        this.doTraverse(visitor, blockScope);
    }
    visitor.endVisit(this, blockScope);
    }

}

On peut alors implémenter TryStatement

 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

public class TryStatement extends ATraverser {

    @Override
    protected void doTraverse(Visitor<ATraverser> visitor, BlockScope blockScope) {

        LocalDeclaration[] localDeclarations = this.resources;
        int i = 0;

        int max;
        for(max = localDeclarations.length; i < max; ++i) {
            localDeclarations[i].traverse(visitor, this.scope);
        }

        this.tryBlock.traverse(visitor, this.scope);
        if(this.catchArguments != null) {
            i = 0;

            for(max = this.catchBlocks.length; i < max; ++i) {
                this.catchArguments[i].traverse(visitor, this.scope);
                this.catchBlocks[i].traverse(visitor, this.scope);
            }
        }

        if(this.finallyBlock != null) {
            this.finallyBlock.traverse(visitor, this.scope);
        }

    }

}

Salut,

Désolé pour le délai de ma réponse mais j'ai eu très peu de temps à moi cette semaine.

@victor: C'est typiquement ce que je fais dans beaucoup de cas mais des vérifications doivent être fait par moment pour créer un autre modèle. Certains méthodes visit de la bibliothèque sont très génériques sur les éléments qu'elles gèrent alors que mon implémentation doit clairement les identifier. Ceci n'est pas gênant, c'est vraiment la nécessité de découpler ma classe en plusieurs que j'aimerais.

@firm1, @Sebajuste: Malheureusement, je n'ai pas la main sur tout ça. Ces implémentations sont imposées par ma bibliothèque.