Password Generator fertig! Bitte um Kritik!

Mechanics

Ich hab jetzt kein Problem mit deinem Qt Code... Und nein, es gibt keine guten (deutschen) Seiten. Im Grunde gibts bei Qt nicht viel zu wissen. Es gibt gewisse Grundlagen, die man kennen muss, wie QObject, Signals/Slots usw... Viel ist es ja nicht. Und der Rest ist einfach nur die Referenz. Und da schaut man einfach in die offizielle Doku, die ist ganz gut. Was es dann vielleicht noch gibt, sind irgendwelche "fortgeschrittenen" Probleme. Aber das ist nichts zum Lernen/Beschreiben, wenn man mal ein konkretes Problem hat und nicht weiß, wie man das am besten löst, versucht man das zu googeln und findet öfter mal Forenbeiträge oder ähnliches, wo das schon mal jemand gelöst hat. Aber sehr wahrscheinlich nicht auf Deutsch.

Ich hab jetzt auch nicht so wirklich ein Problem mit deinem C++ Code... Dafür ist es zu wenig, und wenn da was nicht optimal ist, ist mir das jetzt gar nicht so aufgefallen, bzw. war mir dann schon egal.

Dein Hauptproblem ist aus meiner Sicht, dass du einfach viel zu wenig Ahnung von Softwaredesign hast. Man schreibt keine Software, wie du das gemacht hast. Das kannst du im Endeffekt auch iterativ lösen, glaube ich. Schau dir den ganzen "komischen" Code an und versuch das nach und nach aufzulösen. Keine Redundanzen. Kein if-else...

hustbaer

@b1llyth3k1t sagte in Password Generator fertig! Bitte um Kritik!:

    QVector<QString> Key_Change;
    Key_Change << "A" << "B" << "C" << "D" << "E" << "F" << "G" << "H" << "I" << "J" << "K" << "L" << "M" << "N" << "O" << "P" << "Q" << "R" << "S" << "T" << "U" << "V" << "W" << "X" << "Y" << "Z";
    for(int i = 0; i < Key_Change.size(); i++)
    {
        Keys.append(Key_Change[i]);
    }

Verstehe denn Sinn davon nicht.
Wieso nicht gleich

         Keys << "A" << "B" << "C" << "D" << "E" << "F" << "G" << "H" << "I" << "J" << "K" << "L" << "M" << "N" << "O" << "P" << "Q" << "R" << "S" << "T" << "U" << "V" << "W" << "X" << "Y" << "Z";

Und das...

    if(ui->checkBox->checkState() == 0 && ui->checkBox_2->checkState() == 0 && ui->checkBox_3->checkState() == 0 && ui->checkBox_4->checkState() == 0 && ui->checkBox_5->checkState() == 0)
    {
        QMessageBox msgBox;
        msgBox.setText("Please Setup the Settings!");
        msgBox.exec();
    }

Also erstmal würde ich das eher so schreiben:

    if(Keys.empty())
    {
        QMessageBox msgBox;
        msgBox.setText("Please Setup the Settings!");
        msgBox.exec();
    }

Und dann fehlt da noch ein return; wenn du nicht willst dass dir der Modulo-Operator (%) in der folgenden Schleife um die Ohren fliegt.

Und natürlich

    for(int d = 0; d < number; d++)
    {
        generate_password = generate_password + Keys.at(rand() % Keys.size() + 1);
        ui->lineEdit->setText(generate_password);
    }

Wozu die UI bei jedem Schleifendurchlauf updaten? Und wieso die umständliche "a = a + b" Schreibweise?
=>

    for(int d = 0; d < number; d++)
        generate_password += Keys.at(rand() % Keys.size() + 1);

    ui->lineEdit->setText(generate_password);

Und ... + 1? Huch? Willst du dass dir ein out_of_range rausfliegt?

hustbaer

Dann hätten wir den Code mal mehr oder weniger "sauber". DANN könnten wir uns die ganzen Variablen-Namen ansehen. number eieiei. Oder generate_password ... das sollte wohl eher generated_password heissen.

B1LLYTH3K1T

@hustbaer Vielen Dank ! Das hat mir echt super weiter geholfen !

B1LLYTH3K1T

@mechanics Du hast von Software design geredet irgendwelche guten Bücher dazu ?

B1LLYTH3K1T

@hustbaer sagte in Password Generator fertig! Bitte um Kritik!:

Also erstmal würde ich das eher so schreiben:
if(Keys.empty())
{
QMessageBox msgBox;
msgBox.setText("Please Setup the Settings!");
msgBox.exec();
}

Und dann fehlt da noch ein return; wenn du nicht willst dass dir der Modulo-Operator (%) in der folgenden Schleife um die Ohren fliegt.
Und natürlich
for(int d = 0; d < number; d++)
{
generate_password = generate_password + Keys.at(rand() % Keys.size() + 1);
ui->lineEdit->setText(generate_password);
}

Wozu die UI bei jedem Schleifendurchlauf updaten? Und wieso die umständliche "a = a + b" Schreibweise?
=>
for(int d = 0; d < number; d++)
generate_password += Keys.at(rand() % Keys.size() + 1);
ui->lineEdit->setText(generate_password);
Und ... + 1? Huch? Willst du dass dir ein out_of_range rausfliegt?

Kannst du mir das nochmal genau erläutern?

Mechanics

@b1llyth3k1t sagte in Password Generator fertig! Bitte um Kritik!:

@mechanics Du hast von Software design geredet irgendwelche guten Bücher dazu ?

Auswending nicht, aber da gibts einiges, was man lesen könnte.
Ein bisschen was über Clean Code, C++ Core Guidelines, gibt sicher auch lauter Bücher über allgemeine Prinzipien.
Dann sollte man sich auf jeden Fall mal die Design Patterns anschauen.
Über Software Architektur (weiterführende Patterns, größere Zusammenhänge) gibts auch paar Bücher, ist aber schon Jahre her, dass ich die gelesen habe.

Schau dir doch einfach mal in der Bibliothek einer FH oder Uni bei dir in der Nähe um. Zumindest bei uns in der FH gibts sehr viele Bücher (auch zu diesen Themen), die kann man in Ruhe mal durcharbeiten. Gibt auch sehr viele sehr ähnliche Bücher, irgendwann kann man das gut unterscheiden und dann weiß man, dass 10-20 Bücher wahrscheinlich reichen. Bei uns kann man auch Bücher bestellen lassen, wenn die Bibliothek sie noch nicht hat. Aber für den Anfang reichen wahrscheinlich die Bücher, die schon da sind, und später "stolperst" du dann selber über gute Bücher, die man lesen sollte.

hustbaer

@b1llyth3k1t sagte in Password Generator fertig! Bitte um Kritik!:

Kannst du mir das nochmal genau erläutern?

Was genau verstehst du nicht?

Mechanics

Wahrscheinlich versteht er nicht, dass die Indizes bei 0 anfangen?

hustbaer

@Mechanics
Naja er hat da ziemlich viel zitiert und ich hab nicht vor alles da auf einem Niveau zu erklären dass jemand der überhaupt nichts davon versteht es dann versteht. Weil mir das zu viel Aufwand ist.