Bitte um Refactoring, Passwortgenerator mit Vorgaben

[wob]

Warum returnt dein is_valid eigentlich int statt bool und was testet es genau?

Warum nutzt du intern überall char16_t? Du castest das sowieso am Ende nach char, wenn du es an den string anhängst. Die Frage ist dann eher, wie man das hier Unicode-Kompatibel macht. Mit int32 (Code points) arbeiten oder gleich z.B. utf8 erzeugen?

Warum hat deine alphabet-Klasse eigentlich vector als Member, vo du doch eigentlich nur from und to bräuchtest - und was haben min und max im Alphabet verloren?

Was macht search_char, insbesondere warum ist da irgendwas mit random drin? Klingt erstmal so, als solle nach einem Zeichen gesucht werden, aber schon der String-Parameter verursacht gleich Fragezeichen bei mir.

Warum returnt get_size int? Warum heißt es nicht analog zur STL einfach nur size und returnt einen size_t?

Warum ist keine Klasse und keine Funktion dokumentiert? Ich bin ein Fan davon, dass alle Klassen und öffentlichen Funktionen zumindest einen kurzen Kommentar haben, wo steht, was die Funktion tut. Insbesondere wenn das nicht von Namen schon 100% klar ist.

Warum ist SIZEC ein Makro? Und wofür steht das? Ich finde den Namen nicht selbsterklärend.

Usw.

[Schlangenmensch]

@EinNutzer0 sagte in Bitte um Refactoring, Passwortgenerator mit Vorgaben:

Eine andere Frage: Ist denn for (auto a : all_alphabets) { wenigstens richtig - oder soll ich hier auch die Referenz verwenden: for (auto &a : all_alphabets) {?

for (auto a : all_alphabets) -> In jeder Iteration wird eine Kopie angelegt
for (auto& a : all_alphabets) -> In jeder Iteration wird eine Referenz verwendet

Eine Kopie kann teuer sein, bei einer Referenz kann man das Objekt verändern. Wenn man verhindern möchte, dass über eine Referenz das Objekt verändert wird, sollte man const verwenden.

for (const auto& a : all_alphabets)

Eigentlich sollte man immer const verwenden, wenn das möglich ist.

Es gäbe auch noch die rvalue Referenz

for (auto&& a : all_alphabets)

oder die Universal Referenz

template<typename T> 
void alphabets::validate(T&& s)

Zu erklären, wann man was verwendet und was man dabei beachten muss, dauert länger.

Prinzipiell würde deinem Code ein bisschen mehr const gut tun.

[hiyuck]

Hier sind einige Verbesserungen, die ich vorschlagen würde:

1.Verwende den using-Direktive nicht in der globalen Ebene. Stattdessen solltest du sie innerhalb einer Funktion oder eines Klassenkörpers verwenden.

2.Verwende constexpr statt #define für die SIZEC-Konstante.

3.Statt einer globalen Instanz von mrand, solltest du das random_device und mt19937-Objekte als private Member-Variablen in der mrand-Klasse haben. Dann könntest du einen Konstruktor schreiben, der diese Objekte initialisiert. Auf diese Weise könntest du auch mehrere Instanzen von mrand erstellen, falls dies benötigt wird.

4.Verwende const für Methoden, die den Zustand einer Klasse nicht ändern.

5.Verwende auto statt expliziter Typdeklarationen, wenn möglich.

6.Verwende std::u16string statt char16_t und std::u16string::value_type statt char16_t für den Typ der Elemente in all_chars.

7.Verwende std::array statt std::vector für all_chars, da es wahrscheinlich besser für die Leistung ist und die Größe bei der Compilierzeit festgelegt wird.

8.Füge override hinzu, wenn du Methoden von einer Basisklasse überschreibst.

9.Verwende std::vector::empty statt std::vector::size zum Prüfen, ob all_chars leer ist.

10.Verwende std::find statt std::vector::find zum Suchen von Elementen in all_chars, da es wahrscheinlich besser für die Leistung ist.

11.Verwende std::count_if statt einer Schleife und std::find zum Zählen der Anzahl von Elementen in s, die in all_chars enthalten sind.

12.Verwende std::uniform_int_distribution statt std::uniform_int_distribution<> und verwende std::mt19937::result_type statt int als Typ für die Verteilung.

[EinNutzer0]

Ein(!) neues Benutzerpasswort für den normalen Anwendungszweck zu generieren, ist doch überhaupt keine zeitkritische Angelegenheit... insofern kann ich die Punkte 6. bis 12. aus @hiyuck s Antwort _nicht_ unmittelbar nachvollziehen...

Dennoch Danke für die Reviews + Suggestions (und Ideas).

[SeppJ]

Punkte 6 und 12 haben nix mit Performance zu tun, sondern mit Korrektheit. Da überhaupt nicht ersichtlich ist, wie du überhaupt auf die Idee kommen könntest, dass dies mit Performance zu tun hätte, noch du deinen Gedankengang näher erläuterst, kann man dich auch nicht genauer korrigieren.

[EinNutzer0]

@hiyuck schreibt doch selber, dass die meisten Punkte auf Leistung, Performance und Mikrooptimierung beruhen... @SeppJ

Leider aber alles ohne Begründung dafür.

[Leon0402]

@EinNutzer0 Ich sehe jetzt nicht genau was du meinst. Bei Punkt 7 und Punkt 10 steht was von Leistung. Die beste Begründung ist das hier imo nicht. Aber sonst sehe ich bei 6 und 12 und schon mal gar nicht von 6 bis 12 nur Punkte über Performance.

[SeppJ]

@EinNutzer0 sagte in Bitte um Refactoring, Passwortgenerator mit Vorgaben:

@hiyuck schreibt doch selber, dass die meisten Punkte auf Leistung, Performance und Mikrooptimierung beruhen... @SeppJ

Leider aber alles ohne Begründung dafür.

Citation needed.

Bitte lern' Lesen! Wirklich. Es ist ganz schlimm. Egal ob Antworten auf Fragen hier im Forum oder Handbücher, deine Interpretationen sind stets komplett ausgedacht.

[EinNutzer0]

@hiyuck sagte in Bitte um Refactoring, Passwortgenerator mit Vorgaben:

wahrscheinlich besser für die Leistung ist

Tipp: Strg+F im Browser und "Leistung" eingeben

[NutzerZwei]

Dieser Beitrag wurde gelöscht!

[SeppJ]

Wenn du darauf bestehst, machen wir aus einem temporären Bann, eben einen Permabann.