Salut,
J'ai survolé et je crois qu'il y a d'abord la conception à remanier.
ConfigMonde et Village peuvent se réduire à des structures. Leur constructeur par défaut ne sert à rien.
TribalTime a trop de responsabilité => à refactoriser pour bien séparer les différentes responsabilités. J'ai l'impression qu'elle regroupe à la fois les responsabilités d'affichage, de contrôle et de modèle (cf Modèle/Vue/Contrôleur par exemple)
Pourquoi utiliser des float ?
Tu utilises à plusieurs reprises des int pour gérer du 1/0 => bool.
J'aime pas (et ce n'est pas MISRA par exemple) les définitions multiples sur la même ligne :
1
2
| float vitMonde = ui->vitesseMondeAjout->text().toFloat(),
vitUnite = ui->vitesseUniteAjout->text().toFloat(); |
=>
1
2
| float vitMonde = ui->vitesseMondeAjout->text().toFloat();
float vitUnite = ui->vitesseUniteAjout->text().toFloat(); |
Il y a des constantes 'magiques' : exportation->setGeometry(20, 20, 360, 160); par exemple. Pourquoi 20,20,360,160 ? Ca représente quoi ? Il y a-t-il un rapport entre elles ?
Je n'aime pas les tests à rallonge. Exemples :
1
2
3
| if(ui->xVillageCible->text().toInt() > 0 && ui->yVillageCible->text().toInt() > 0 && ui->xVillagePersonnel->text().toInt() > 0 && ui->yVillagePersonnel->text().toInt() > 0)
if(numMonde > 0 && vitMonde > 0 && vitUnite > 0)
if(nomVillage.length() > 0 && xVillage > 0 && xVillage <= 1000 && yVillage > 0 && yVillage <= 1000 && (groupeVillage == 0 || groupeVillage == 1)) |
On ne sait pas dire quel est l'objectif du test.
Je me méfie des constructions : while(std::getline(listeMondes, ligneFichier) && stop == 0). En général la seconde condition de sorties est soit parce que le parcours ne devait pas être complet soit une gestion cachée d'erreur. Voir break (ou return si la responsabilité a été isolée dans une fonction dédiée) dans le premier cas, et ne pas avoir peur des exceptions.
Ou quelque chose m'échappe, ou :
1
2
3
4
5
6
7
8
|
for(my_tok::const_iterator i = tok.begin(); i != tok.end(); ++i)
{
if(a == 0)
element = QString::fromStdString(*i).toInt();
a++;
} |
doit pouvoir se réduire plus clairement à
1
2
3
4
5
6
|
my_tok::const_iterator i = tok.begin()
if(i != tok.end())
{
element = QString::fromStdString(*i).toInt();
} |
Je pense qu'en construisant un itérateur sur la lecture des enregistrements du fichier permettrait d'écrire une partie des fonctions à l'aide des algos de la STL de façon plus expressives (tout les while(std::getline(listeVillages, ligneFichier) && stop == 0))
Tu dois pouvoir utiliser des tuples pour éviter les boucles avec la variable a sur les chaînes découpées en token.
QDateTime heureArrivee = heureDepart.addSecs(-(ceil(60*(tempsUnites[ui->selectionUniteLente->currentIndex()]/(this->vitesseMonde*this->vitesseUnite))*nbCases)+delais));
... Une expression comme ça, je ne sais pas dire si la formule est valide, si les variables utilisées dans la formule sont les bonnes, si les types sont bons.
ui devrait être géré dans un unique_ptr.
unites et tempsUnites sont des données statiques, connues à la compilation et reconstruites à chaque fois. C'est dommage.
Tu n'as pas besoin de this-> pour accéder aux variables membres.
Voilà pour une première volée de remarques. Je pense qu'il y en a encore d'autres.
Sinon, les noms semblent bien appropriés. Ca facilite grandement la lecture et ça aide à comprendre l'objectif de la variable/fonction/classe/etc.. 
Portus 
Conception et optimisation de code
Répondre avec citation
Envoyé par Portus
Partager