Discussion :

[joel.drigo]

Salut,

A mon avis, les 2 premiers "niveaux" de splitCcPairSymbo ne servent à rien : les supprimer permettrait de gagner un peu de cpu (un peu de traitement de pile, et gain sur les trucs faits en double).

Je remplacerais la termMapByString, map étant fixe visiblement, par un switch. Aussi fastidieux à écrire, mais probablement un peu plus lisible en plus. Tu peux faire une moulinette pour la conversion si tu ne veux pas tout te retaper.

[hwoarang]

Deja, tu n'utilises pas map.putIfAbsent. A mon avis, ce serait mieux pour s'assurer que c'est toujours le meme objet cpair qui est utilisé. Je sais que tu as deja dit que ce n'etait pas grave pour toi mais c'est contre intuitif quand on crée un cache d'utiliser des objets qui ne viennent pas du cache. Si demain, quelqu'un d'autre touche a ton code, il va peut etre vouloir mettre une instru dans l'objet CCPair et ne s'attendra pas à ce que des objets qui ne sont pas cachés soient utilisés.

1. De ce que j'ai compris du post, un split sur un char n'utilise pas de regex ?

Vu la simplicité de ce que tu fais, en supposant que ca a reellement un impact sur ton fonctionnement, je partirais plutot sur un parsing de la string à la main. Une regex a besoin d'etre compilée et interprétée. C'est puissant pour des traitements relativement complexes mais dans ton cas, ca me parait etre la grosse artillerie pour pas grand chose. Une petite boucle de quelques millions de tests te permettra de voir si ca t'apporte quelque chose ou non.

2. En allant voir le code de get() pour une concurrentHashMap je n'ai pas vu de synchronize contrairement au put(). Etes vous sur que le get est egalement synchronize comme dit plus haut ?

C'est justement l'interet d'utiliser une concurrentHashMap. Seul le put est synchronisé (voir la javadoc). Le get n'est synchronisé que si l'item est en cours d'insertion.

[adiGuba]

Vu la simplicité de ce que tu fais, en supposant que ca a reellement un impact sur ton fonctionnement, je partirais plutot sur un parsing de la string à la main. Une regex a besoin d'etre compilée et interprétée. C'est puissant pour des traitements relativement complexes mais dans ton cas, ca me parait etre la grosse artillerie pour pas grand chose. Une petite boucle de quelques millions de tests te permettra de voir si ca t'apporte quelque chose ou non.

Pas forcément : le split() de la classe String n'utilise pas les regexp lorsqu'on lui passe un séparateur d'une seule lettre...

Par contre oui dans ce cas précis comme on fait déjà des indexOf() autant faire directement un substring() derrière.

2. En allant voir le code de get() pour une concurrentHashMap je n'ai pas vu de synchronize contrairement au put(). Etes vous sur que le get est egalement synchronize comme dit plus haut ?

Parce qu'un code thread-safe ne passe pas forcément par de la synchronisation, notamment pour la lecture.
En travaillant avec des données volatile et/ou des opérations atomiques, on peut très bien faire un code thread-safe sans forcément avoir besoin de bloc synchronized.




Sinon deux petites remarques sur ton code :

• "termMapByString" et "termMapByFixTenor" sont deux Map static non-threadsafe.
  Cela ne pose pas de problème puisque tu ne les modifies pas après-coup, pour l'instant...
  Si un jour quelqu'un s'amuse à modifier ces Map après l'initialisation de la classe, tu pourrais te retrouver avec des erreurs difficiles à comprendre.
  
  Je te conseille de les englober dans une "unmodifiableMap".
  Perso dans ce cas j'utilise le pattern suivant :
  

  Code :

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

  1
  2
  3
  4
  5
  6
  7
  	private static final Map<String, Term> termMapByString = createTermMapByString();
   
  	private static Map<String, Term> createTermMapByString() {
  		Map<String, Term> map = new HashMap<>();
  		// map.put(...);
  		return Collections.unmodifiableMap(map);
  	}

  Voir si tu ne veux pas définir de méthode même private :
  

  Code :

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

  1
  2
  3
  4
  5
  6
  	private static final Map<String, Term> termMapByString;
  	static {
  		Map<String, Term> map = new HashMap<>();
  		// map.put(...);
  		termMapByString = Collections.unmodifiableMap(map);
  	}

  
  
• Pourquoi splitCcPairSymbol() retourne-t-il une List<String> ?
  Cela me semble un peu inutile puisque tu construis une liste pour la décomposer immédiatement et créer un CCPair.
  Autant faire en sorte que cette méthode retourne directement un CCPair, tu t'éviteras encore quelques objets temporaires...

a++

[tchize_]

2. En allant voir le code de get() pour une concurrentHashMap je n'ai pas vu de synchronize contrairement au put(). Etes vous sur que le get est egalement synchronize comme dit plus haut ?

Une erreur de ma part que hwoarang a déjà corrigée. J'ai en fait mal lu ta réponse et j'ai cru que tu utilisais Collections.synchronizedMap(). Désolé.

[Yonito]

Sinon deux petites remarques sur ton code :

• "termMapByString" et "termMapByFixTenor" sont deux Map static non-threadsafe.
  Cela ne pose pas de problème puisque tu ne les modifies pas après-coup, pour l'instant...
  Si un jour quelqu'un s'amuse à modifier ces Map après l'initialisation de la classe, tu pourrais te retrouver avec des erreurs difficiles à comprendre.
  
  Je te conseille de les englober dans une "unmodifiableMap".
  Perso dans ce cas j'utilise le pattern suivant :
  

  Code :

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

  1
  2
  3
  4
  5
  6
  7
  	private static final Map<String, Term> termMapByString = createTermMapByString();
   
  	private static Map<String, Term> createTermMapByString() {
  		Map<String, Term> map = new HashMap<>();
  		// map.put(...);
  		return Collections.unmodifiableMap(map);
  	}

  Voir si tu ne veux pas définir de méthode même private :
  

  Code :

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

  1
  2
  3
  4
  5
  6
  	private static final Map<String, Term> termMapByString;
  	static {
  		Map<String, Term> map = new HashMap<>();
  		// map.put(...);
  		termMapByString = Collections.unmodifiableMap(map);
  	}

  
  
• Pourquoi splitCcPairSymbol() retourne-t-il une List<String> ?
  Cela me semble un peu inutile puisque tu construis une liste pour la décomposer immédiatement et créer un CCPair.
  Autant faire en sorte que cette méthode retourne directement un CCPair, tu t'éviteras encore quelques objets temporaires...

a++

Merci ce conseil la , je vais l'utilise dans mon code !