Discussion :

[joel.drigo]

Salut,

je viens de rencontrer un problème de types paramétrés qui m'a obligé à mettre en place une solution qui ne me plait pas. Mais j'ai beau chercher, je n'arrive pas à trouver comment obtenir une solution satisfaisante.

En résumé, j'ai tout d'abord une interface :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public interface IViewItem<O> {...}

J'ai dans un composant une méthode comme ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
public void expand(Collection<IViewItem<?>> objects, int level) {
   if ( objects!=null ) {
      if ( level<0 ) level = TreeBasketViewer.ALL_LEVELS;
         for(Object object : objects) {
            getViewer().expandToLevel(object, level); // getViewer() me renvoit du org.eclipse.jface.viewers.TreeViewer
      }
   }
}

J'ai une action qui appelle cette méthode à partir d'une sélection, donc un objet de classe implémentant l'interface org.eclipse.jface.viewers.ISelection.
Comme les implémentations de cette interface ne me renvoit que des Object[] ou des List (sans paramètre), je me suis fait une classe d'utilitaire avec une méthode pour obtenir une collection paramétrée, ce qui facilite le traitement en amont, comme suit, qui me permet en plus d'extraire uniquement les objets qui correspondent à la classe spécifiée :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
public static <T> Collection<T> getSelection(ISelection selection, Class<T> clazz) {
   Collection<T> col = new ArrayList<T>();
   return _getSelection(selection, col, clazz, false);
}

En général, elle me sert à récupérer dans une sélection d'instances de type différents, les objets d'un type donné et je peux ainsi facilement écrire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
for(Page page : SelectionUtil.getSelection(selection, Page.class)) {
   if ( page.getFolio().isOdd() )  {
       ...
   }
}

Maintenant pour appeler ma méthode expand, j'ai une action avec le code suivant :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
@SuppressWarnings("unchecked")
@Override
public void run() { 
   BasketSelection basketSelection = (BasketSelection) getBasketHandler().getSelection();
   Collection<? extends IViewItem<?>> tmpobjects = (Collection<? extends IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);
   Collection<IViewItem<?>> objects = new ArrayList<IViewItem<?>>(tmpobjects);
   switch(state) {
   case ONE_LEVEL:
      basketHandler.expand(objects,1);
      break;
   case ONE_LEVEL_MORE:
       basketHandler.expandMore(objects,1);
       break;
   case ALL_LEVELS:
       basketHandler.expand(objects,TreeBasketViewer.ALL_LEVELS);
       break;
   }
}

Elle fonctionne mais, ce qui ne me plait pas, c'est le fait de créer une nouvelle ArrayList (le code en rouge), de dupliquer en fait ma collection d'origine. Et en plus de devoir caster en Collection<? extends IViewItem<?>>. Ça ressemble a de la bidouille.

SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class); retourne du Collection<IViewItem> qui ne peut être casté en Collection<IViewItem<?>>, type attendu par la méthode expand().

J'ai essayé de modifier ma méthode SelectionUtil.getSelection(...) avec : Collection<? super T>, Collection<? extends T>, mais j'ai toujours un problème au final lors de l'appel de expand(...).
Evidemment, pour getSelection(), le type T peut être n'importe quelle classe, paramétrée ou pas paramétrée.

Si je fais :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
Collection<IViewItem> objects = SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);
switch(state) {
case ONE_LEVEL:
   basketHandler.expand(objects,1);
   break;
   ...
}

ça ne compile pas parce que objects devrait être du Collection<IViewItem<?>>...

Si je fais :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
Collection<IViewItem> objects = SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);
switch(state) {
case ONE_LEVEL:
   basketHandler.expand((Collection<IViewItem<?>>)objects,1);
   break;
   ...
}

Je ne peux évidemment pas compiler à cause du fait que je ne peux pas caster du Collection<IViewItem> en Collection<IViewItem<?>>

Je ne peux pas écrire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

Collection<IViewItem<?>> objects = SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

Je ne trouve pas de solution...

[tchize_]

Quelques remarques, car je ne connais pas eclipse et je ne suis pas sur de tout comprendre

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public void expand(Collection<IViewItem<?>> objects, int level

A aucun moment, dans cette méthode, tu n'appelle de méthode nécessitant de passer un IViewItem<?> en paramètre. Du coup, cette méthode peux très bien être converties en

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public void expand(Collection<? extends IViewItem<?>> objects, int level)

Avec l'avantage qu'on peux lui passer aussi bien une
Collection<? extends IViewItem<?>> que une Collection<IViewItem<?>> ou encore un List<TrucMuch> (pour autant que TrucMuch extends IViewItem)

Ensuite, ce code:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
public static <T> Collection<T> getSelection(ISelection selection, Class<T> clazz) {
   Collection<T> col = new ArrayList<T>();
   return _getSelection(selection, col, clazz, false);
}

Me semble curieux. Pourquoi ne pas avoir mis plutôt en place un ISelection<T> qui retourne déjà comme un grand une liste bien typée, plutot que de bidouiller après coup?

Enfin, ce code:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

Collection<? extends IViewItem<?>> tmpobjects = (Collection<? extends IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

m'a l'air suspect. Si j'en juge par le code de getSelection, ca te retourne une Collection<IViewItem> pas une Collection<? extends IViewItem>. du coup, tu cast en "plus large" et après tu bidouille avec une nouvelle arraylist pour compenser. Tu aurais pu écrire directement

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

   Collection<IViewItem<?>> tmpobjects = (Collection<IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

Enfin, si on s'affranchis du clazz de getSelection() (est-il vraiment utile), on aurais pu faire:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
public static <T> Collection<T> getSelection(ISelection selection) {
   Collection<T> col = new ArrayList<T>();
   return _getSelection(selection, col, false);
}
 
Collection<IViewItem<?>> objects = SelectionUtil.getSelection(basketSelection.getItemSelection());

[joel.drigo]

Quelques remarques, car je ne connais pas eclipse et je ne suis pas sur de tout comprendre

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public void expand(Collection<IViewItem<?>> objects, int level

A aucun moment, dans cette méthode, tu n'appelle de méthode nécessitant de passer un IViewItem<?> en paramètre. Du coup, cette méthode peux très bien être converties en

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public void expand(Collection<? extends IViewItem<?>> objects, int level)

Oui, c'est peut être ça la solution : je vais essayer ça

Collection<? extends IViewItem<?>> que une Collection<IViewItem<?>> ou encore un List<TrucMuch> (pour autant que TrucMuch extends IViewItem)

Ensuite, ce code:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
public static <T> Collection<T> getSelection(ISelection selection, Class<T> clazz) {
   Collection<T> col = new ArrayList<T>();
   return _getSelection(selection, col, clazz, false);
}

Me semble curieux. Pourquoi ne pas avoir mis plutôt en place un ISelection<T> qui retourne déjà comme un grand une liste bien typée, plutot que de bidouiller après coup?

Parce que ISelection est une interface de JFACE.

Enfin, ce code:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

Collection<? extends IViewItem<?>> tmpobjects = (Collection<? extends IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

m'a l'air suspect. Si j'en juge par le code de getSelection, ca te retourne une Collection<IViewItem> pas une Collection<? extends IViewItem>. du coup, tu cast en "plus large" et après tu bidouille avec une nouvelle arraylist pour compenser. Tu aurais pu écrire directement

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

   Collection<IViewItem<?>> tmpobjects = (Collection<IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

Compile pas justement : Eclipse me dit que j'ai pas le droit de caster du Collection<IViewItem> en Collection<IViewItem<?>>. C'est bien là le principal problème.

Enfin, si on s'affranchis du clazz de getSelection() (est-il vraiment utile), on aurais pu faire:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
public static <T> Collection<T> getSelection(ISelection selection) {
   Collection<T> col = new ArrayList<T>();
   return _getSelection(selection, col, false);
}
 
Collection<IViewItem<?>> objects = SelectionUtil.getSelection(basketSelection.getItemSelection());

Le but de la méthode getSelection, ce n'est pas seulement d'obtenir une Collection<T> mais aussi de filtrer sur la classe.

Si j'ai dans ma sélection des objects de classe A et de classe B sans autre lien que la classe Object, et que je veux traiter mes éléments sélectionnés de classe A, sans avoir à faire un machin du genre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
for(Object object : selection.toList() ) {
    if( object instanceof A ) {
        A a = (A)object;
        ...
    }
}

je préfère écrire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
for(A a : SelectionUtil(selection, A.class) ) {
    ...
}

J'ai besoin du clazz pour faire le instanceof, mais en plus ma méthod _getSelection traite l'adaptation en option :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
private static <T> Collection<T> _getSelection(
			ISelection selection, Collection<T> col, Class<T> clazz, boolean adapted) {
		if ( selection==null || selection.isEmpty() ) {
		}
		else if ( selection instanceof IStructuredSelection ) {
			IStructuredSelection ssel = (IStructuredSelection) selection;
			List list = ssel.toList();
			if ( list!=null ) {
				for(Object object : list) {
					if ( clazz.isInstance( object ) ) {
						col.add((T) object);
					}
					else if ( adapted && object instanceof IAdaptable ) {
						IAdaptable adaptable = (IAdaptable) object;
						Object adaptedInstance = adaptable.getAdapter(clazz);
						if ( clazz.isInstance( adaptedInstance ) ) {
							col.add((T) object);
						}
					}
				}
			}
		}
		return col;
	}

[joel.drigo]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public void expand(Collection<IViewItem<?>> objects, int level

A aucun moment, dans cette méthode, tu n'appelle de méthode nécessitant de passer un IViewItem<?> en paramètre. Du coup, cette méthode peux très bien être converties en

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public void expand(Collection<? extends IViewItem<?>> objects, int level)

Donc, c'était bien là la solution. Moi qui cherchais en amont.

Je suis quand même obligé de caster mon retour de getSelection() :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
Collection<? extends IViewItem<?>> objects = (Collection<? extends IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);
switch(state) {
   case ONE_LEVEL:
      basketHandler.expand(objects,1);
      break;
   case ONE_LEVEL_MORE:
      basketHandler.expandMore(objects,1);
      break;
   case ALL_LEVELS:
      basketHandler.expand(objects,TreeBasketViewer.ALL_LEVELS);
      break;
}

Mais c'est déjà quand même plus clean.



Je n'ai pas besoin, en effet, dans l'implémentation que j'ai postée, de traiter directement du IViewItem<?>, mais je le fais dans d'autres méthodes, de même famille fonctionnelle, dans la même classe, et je voulais avoir uniformité des signatures. Mais je traite le cas en aval, donc ça ne pose pas de problème d'avoir un argument de type Collection<? extends IViewItem<?>>.

Merci @Tchize_

[tchize_]

un moyen de faire ton filtrage sans que ton clazz soit dans le chemin (c'est à cause de lui que t'es obligé de caster, car on ne sais pas facilement faire en java un Class<IViewItem<?>> ), ce serait de passer une instance de filtrage, un truc du genre

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
public interface SelectionFilter<T>{
   // return o as a T, or null if impossible
   public T convert(Object o){
   }
}
public interface IViewItemFilter extends SelectionFilter<IViewItem<?>>{
   public IViewItem<?> convert(Object o){
      if (o instanceOf IViewItem)
          return (IViewItem)o;
      return null;
   }
}

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
private static <T> Collection<T> _getSelection(
			ISelection selection, Collection<T> col, SelectionFilter<T> filter, boolean adapted) {
		//.....
				for(Object object : list) {
                                        T t = filter.convert(object);
					if ( t!=null ) {
						col.add((T) object);
					}

[joel.drigo]

Oui, ça pourrait être une évolution intéressante de la classe pour des cas de filtrage plus évolués qu'un simple filtrage sur la classe, ou adaptation sur la classe (du coup je n'aurais même plus besoin du booléen adapted, je pourrais gérer ça dans mon implémentation de filtre). Je n'en ai pas eu besoin pour le moment, mais ça pourrait arriver effectivement.

Mais, j'ai genre 250 appels de la fonction qui marchent très bien : ce serait dommage d'avoir à modifier le système juste pour 1 nouvel appel. Et puis, ça me gène toujours d'avoir à instancier une classe, surtout en anonyme, quand je peux m'en passer. Surtout que j'ai une dizaine de méthodes de convenience qui doivent suivre


Le jour ou je passerai en Java 8, genre en 2025 je pourrais au moins utiliser les lambdas pour faire mon filtrage sans faire de classe anonyme.

[joel.drigo]

Bon, bah, je l'ai fait finalement, l'implémentation avec classe de filtre. J'ai laissé mes méthodes de convenience qui instancient juste une classe de filtre adaptée à chaque fois.

Et pour mon cas particulier, j'ai un singleton pour le filtre, donc pas d'instanciation de classe anonyme à chaque appel.

Du coup, j'ai un bénéfice : je n'ai plus de @unchecked !

Merci beaucoup pour les tuyaux, @Tchize_

[joel.drigo]

Enfin, ce code:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

Collection<? extends IViewItem<?>> tmpobjects = (Collection<? extends IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

m'a l'air suspect. Si j'en juge par le code de getSelection, ca te retourne une Collection<IViewItem> pas une Collection<? extends IViewItem>. du coup, tu cast en "plus large" et après tu bidouille avec une nouvelle arraylist pour compenser. Tu aurais pu écrire directement

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

   Collection<IViewItem<?>> tmpobjects = (Collection<IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

Sinon, je rebondis sur ta remarque : tout mon problème a bien commencé là. Je m'attendais également à pouvoir écrire tout simplement :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

   Collection<IViewItem<?>> tmpobjects = (Collection<IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);

Mais ça ne compile pas (Comme je le disais, je n'aurais pas le droit de caster du Collection<IViewItem> en Collection<IViewItem<?>>. D'où mon étonnement et ma tentative de corriger ça, avec une bidouille au final parce que je ne trouvais aucune solution.

Est-ce que ce qui pourrait expliquer ça est que je suis sous Java 5 et que ce cas n'a été géré que sous une version plus récente de Java ?

[joel.drigo]

J'ai pu modifier mon code pour éviter de créer une ArrayList supplémentaire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
Collection<? extends IViewItem<?>> tmpobjects = (Collection<? extends IViewItem<?>>) SelectionUtil.getSelection(basketSelection.getItemSelection(), IViewItem.class);
Collection<IViewItem<?>> objects = (Collection<IViewItem<?>>) tmpobjects;

Assez chelou de devoir caster 2 fois, non ?