Besoin d'un avis, générateur de mot de passe

Jo -  
cptpingu Messages postés 3840 Date d'inscription   Statut Modérateur Dernière intervention   -
Bonjour,

je viens de bricoler un petit générateur de mot de passe (code ci dessous)

Je défini plusieurs type possible (lettre minuscule, majuscule, chiffre, minusc + chiffre, majusc + chiffre etc etc)
Etant débutant en C++ (j'y ai passé l'aprem !), j'aurais voulu avoir des avis de personnes plus qualifiés sur les possibles améliorations, et les quelques bonnes habitudes à prendre 

#include <string>
#include <ctime>
#include <cstdlib>

using namespace std;


int main()

{
int longueur, choix;
string lowercase, uppercase, numeric, lower_upper, lower_numeric, upper_numeric, lower_upper_numeric;

cout << "=====  Generateur de mot de passe  =====" << endl;
cout << "1.     lowercase" << endl;
cout << "2.     uppercase" << endl;
cout << "3.     lower_uppercase" << endl;
cout << "4.     numeric" << endl;
cout << "5.     lower_numeric" << endl;
cout << "6.     upper_numeric" << endl;
cout << "7.     lower_upper_numeric" << endl << endl;

cin >> choix;



srand(time(NULL)); // Demarrage de génération aléatoire

switch(choix)

{


case 1:

lowercase = "abcdefghijklmnopqrstuvwxyz";
cout << "Quelle longueur de password ?" << endl;

cin >> longueur;


for (int i = 0 ; i < longueur ; i++)

{
int aleatoire;
aleatoire = rand()%26;

cout << lowercase[aleatoire];
}

break;

case 2:

uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
cout << "Quelle longueur de password ?" << endl;

cin >> longueur;


for (int i = 0 ; i < longueur ; i++)

{
int aleatoire = 0;
aleatoire = rand()%26;

cout << uppercase[aleatoire];
}
break;

case 3:

    lower_upper = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
    cout << "Quelle longueur de password ?" << endl;

    cin >> longueur;


for (int i = 0 ; i < longueur ; i++)

{
int aleatoire = 0;
aleatoire = rand()%52;

cout << lower_upper[aleatoire];
}
break;

case 4:

    numeric = "0123456789";
    cout << "Quelle longueur de password ?" << endl;

cin >> longueur;


for (int i = 0 ; i < longueur ; i++)

{
int aleatoire = 0;
aleatoire = rand()%10;

cout << numeric[aleatoire];
}
break;

case 5:

    lower_numeric = "abcdefghijklmnopqrstuvwxyz0123456789";
       cout << "Quelle longueur de password ?" << endl;

cin >> longueur;


for (int i = 0 ; i < longueur ; i++)

{
int aleatoire = 0;
aleatoire = rand()%36;

cout << lower_numeric[aleatoire];
}
break;

case 6:

    upper_numeric = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
          cout << "Quelle longueur de password ?" << endl;

cin >> longueur;


for (int i = 0 ; i < longueur ; i++)

{
int aleatoire = 0;
aleatoire = rand()%36;

cout << upper_numeric[aleatoire];
}
break;


case 7:

    lower_upper_numeric = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
              cout << "Quelle longueur de password ?" << endl;

cin >> longueur;


for (int i = 0 ; i < longueur ; i++)

{
int aleatoire = 0;
aleatoire = rand()%62;

cout << lower_upper_numeric[aleatoire];
}
break;

default:
    cout << "ERROR";
    break;

}

return 0;
}
A voir également:

1 réponse

Réponse 1 / 1
cptpingu Messages postés 3840 Date d'inscription   Statut Modérateur Dernière intervention   2
 
Bonjour.

Tout d'abord, quelques conseils:

Au niveau de la technique:
  • Quand tu vois qu'une action est répétée X fois dans ton programme à l'identique, c'est que tu peux la "factoriser". Pas besoin de demander la longueur dans chaque cas. Tu demandes la longueur avant d'appliquer le choix avec ton switch.
  • Plutôt que d'écrire chacun des choix avec un switch, tu peux mettre chacun de tes groupes de mots de passe dans un tableau. Ainsi, tu n'as qu'à choisir un élément du tableau pour choisir un groupe. Ça te prend moins de place qu'un switch et c'est plus facile d'ajouter de nouveaux groupes.



Je te propose ma version (un peu plus courte que la tienne :p): 
#include <iostream>
#include <string>
#include <ctime>
#include <cstdlib>

namespace // namespace anonyme, si tu n'es pas à l'aise avec, ignore cette directive
{
  const std::string passwords[] = // constante globale. Une variable globale c'est ultra dégueux, mais une constante c'est parfaitement propre. Le [] veut dire que la taille s'adapte automatiquement.
    {
      "abcdefghijklmnopqrstuvwxyz",
      "ABCDEFGHIJKLMNOPQRSTUVWXYZ",
      "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ",
      "0123456789",
      "abcdefghijklmnopqrstuvwxyz0123456789",
      "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789",
      "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
    };
} // namespace

int main()
{
  srand(time(0));

  std::cout << "=====  Generateur de mot de passe  =====\n"
    "1.     lowercase\n"
    "2.     uppercase\n"
    "3.     lower_uppercase\n"
    "4.     numeric\n"
    "5.     lower_numeric\n"
    "6.     upper_numeric\n"
    "7.     lower_upper_numeric\n"
     << std::endl;

  int choix = 0;
  std::cin >> choix;
  if (choix < 1 || choix > 7)
  {
    std::cerr << "Choix invalide: " << choix << std::endl;
    return 1;
  }

  int longueur = 0;
  std::cout << "Quelle longueur de password: ";
  std::cin >> longueur;

  for (int i = 0; i < longueur; ++i)
  {
    const int aleatoire =  rand() % passwords[choix - 1].size();
    std::cout << passwords[choix - 1][aleatoire];
  }
  std::cout << std::endl;

  return 0;
}


On pourrait en faire encore un peu plus pour rendre le programme plus facile à améliorer, mais je ne veux pas trop complexifier le code.
0
Jo
 
Merci pour ces conseils, je vais me pencher sur les namespaces, en revanche sans namespace std, je dois rajouter à chaque fois std:: :( un peu redondant
En revanche pour le srand((time)NULL) j'avais cru comprendre que c'était un aléatoire plus au pif que si on met 0 (pour une raison de calcul de point de départ du random), je fouillerai l'idée en tout cas.

C'est vrai que mon code est un peu plus dans le style "gros sabot", j'ai pas mal de chose à apprendre

Merci à toi
0
cptpingu Messages postés 3840 Date d'inscription   Statut Modérateur Dernière intervention   2 > Jo
 
sans namespace std, je dois rajouter à chaque fois std:: :( un peu redondant

Si ça s'appelle std:: et non standard:: c'est qu'il y a une raison :). Ce n'est pas redondant, le C++ étant un langage contextuel, sur de gros code tu seras content de pouvoir différencier d'un seul coup d'oeil à qui appartient tel morceau de code. Exemple: std::string, boost::string, my::string => Tu vois immédiatemment quel type de string c'est. En revanche, si je te met "string" tout court, quelle implémentation utilises-tu (std ? boost ? my ? autre ?) ? Sur un gros code tu vas devoir chercher (parfois dans d'autres fichiers) quel est le type de string utilisé (et encore ce n'est qu'un exemple simple parmi d'autres). Le pire étant qu'un using namespace peu introduire des subtilités extrêmement difficile à débugger, même pour un expert (lis le lien que je t'ai filé). Bref, je te conseille très très vivement de ne pas utiliser cette directive, et de prendre les bonnes habitudes dès maintenant.

En revanche pour le srand((time)NULL) j'avais cru comprendre que c'était un aléatoire plus au pif que si on met 0

Rien à voir. En C++ NULL et 0 c'est exactement et strictement la même chose.
Quand tu écris NULL, le compilateur remplace par 0. La raison pour laquelle on n'utilise pas NULL en C++ est que l'on veut éviter certains petits "pièges" qui ne paraissent pas logique au premier abord. Dans les dernières versions de C++, le problème a été corrigé grâce au nouveau mot clé "nullptr". Pour comprendre le rapport entre 0 et NULL, voir: http://0217021.free.fr/portfolio/axel.berardino/articles/null-en-cpp/

time(0) (ou time(NULL), c'est pareil), veut dire: prendre le timestamp courant (en gros le chiffre représentant le nombre de secondes écoulées). "srand" veut un entier d'initialisation et on lui donne le temps courant ce qui augmente les chances que ce soit bien aléatoire. Essaie de faire un srand(1234) et tu verras que tu obtiendras toujours les mêmes résultats.
Néanmoins, si tu veux faire du C++ moderne et que ton compilateur le gère, srand et rand sont des vieilles fonctions à éviter, au profit de "std::random_device" qui est beaucoup plus fiable.
0
Jo
 
Quelques améliorations, j'ai créé une fonction qui s'occupe de faire un random sur les string, c'est donc un peu moins redondant de type : 



int mdprandom(string chaine, int longueur)

{

srand(time(0)); // Demarrage de génération aléatoire

  for (int i = 0 ; i < longueur ; i++)

{
int lettre = 0;
lettre = rand()%chaine.size();

cout << chaine[lettre];
}



Par contre pas moyen de me débarasser du switch

p.s l'utilisation de cout << au final c'est comme en BASIC sur une calculette casio ! (je m'amusais bien à l'époque)
0
cptpingu Messages postés 3840 Date d'inscription   Statut Modérateur Dernière intervention   2
 
Plusieurs soucis avec la fonction que tu proposes:
  • La valeur de retour n'a pas d'intérêt
  • La chaine "chaine" est copiée pour rien (devrait être const std::string& chaine, sans le "&" il y a copie !)
  • srand est appelé à chaque fois, alors qu'un srand ne doit être appelé qu'une seule fois dans ton programme (généralement au début de ton "main").
  • La fonction affiche le mot de passe au lieu de le renvoyer, donc il faudrait soit qu'elle renvoie une chaine, soit qu'elle l'affiche dans un flux donné en argument.


Exemple: 
void mdprandom(const std::string& chaine, int longueur, std::ostream& out)
{
  for (int i = 0; i < longueur; ++i)
    out << chaine[rand() % chaine.size()];
}

int main()
{
  srand(time(0));
  mdprandom("abcdefghijklmnopqrstuvwxyz", 10, std::cout);
  reutrn 0;
}


Par contre pas moyen de me débarasser du switch


En t'aidant du code que j'ai posté, il devrait être aisé de s'en débarasser.
0

Discussions similaires

Mot de passe administrateur Freebox : où le trouver ?
Utilisateur anonyme -
Marie -

7 réponses
Trouver le mdp administrateur freebox ?
Cousyn -
Pierr10 -

1 réponse