-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ajout limite taille liste_ventes et liste_achats envoyé par le serveur #14
Conversation
J'ai essayé de corriger les exceptions que tu as trouvé (9d82199), il faut donc que tu adaptes un peu ton code. |
Sinon sur le code proposé, tu peux juste définir la valeur par défaut de nbMaxElemListe à 0 : |
Oui ça marche ! Je vais mettre mes modifs dans mon pull request. |
Voilà c'est fait. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quelques détails mineurs à améliorer.
2017/SimBourse/src/core/Marche.java
Outdated
mutex_ordre_read.lock(); | ||
String r = String.valueOf(liste_achats.get(a)); | ||
String r; | ||
if(nbMaxAchats<=0 || nbMaxAchats>liste_achats.get(a).size()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tu peux transformer le > strict en >= ici.
2017/SimBourse/src/core/Marche.java
Outdated
if(nbMaxAchats<=0 || nbMaxAchats>liste_achats.get(a).size()) | ||
r = String.valueOf(liste_achats.get(a)); | ||
else | ||
r=String.valueOf(new LinkedList<Ordre>(liste_achats.get(a)).subList(0, nbMaxAchats)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En effet l'opération de création de liste à partir du set coûte n opérations, puis à nouveau nbMaxAchats opérations. Tu peux adopter la solution que tu avais proposé pour limiter la taille de l'historique qui coûtera moins cher. Le mieux serait même de regrouper le tout dans une fonction et faire les 3 appels (ventes, achats, historique) si c'est possible (attention à bien garder l'ordre cependant).
private String SubSetVersString(Set set, int taille)
@@ -114,18 +114,21 @@ public void run() { | |||
envoyer(out, begin + MapToStringPython(joueur.getSolde_actions()) + "}"); | |||
} else if (userInput.startsWith(OPERATIONS) && peut_jouer) { | |||
envoyer(out, String.valueOf(ListPairToStringPythonKeyOnly(joueur.getOperationsOuvertes()))); | |||
} else if (userInput.startsWith(ACHATS) && arguments.length == 2 && StringUtils.isNumeric(arguments[1])&& peut_jouer){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dans l'optique d'optimiser les messages réseaux, tu pourrais garder l'ancien code (ACHATS+ACTION) et ajouter un autre else if indépendant (ACHATS+ACTION+NOMBRE). Comme ça s'il veut récupérer la liste de tous les achats/ventes : il n’envoie pas le nombre pour rien. Mais bon c'est vraiment du détails.
2017/client.py
Outdated
return -4 | ||
#on envoie le numero de l'action | ||
self.__envoyer(self.__message["ACHATS"]+str(numAction)) | ||
self.__envoyer(self.__message["ACHATS"]+str(numAction)+" "+str(nbMaxElemListe)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour faire suite à la remarque précédente, si nbMaxElemListe vaut 0, pas besoin de l'envoyer.
Voilà j'ai refait quelques modifications, maintenant la complexité de la création de la sous liste est en O(nbMaxAchats) je crois. |
C'est bien comme ça, merci. |
C'est le code que j'ai, celui avec lequel j'ai les issues que j'ai envoyé (je les avais aussi avant modifications)