Meinung zu dieser strip/trim Funktion für strings
-
Hallo,
würde mich interessieren, was Eure Meinung zu dieser Funktion ist, und wie gut/schlecht sie zu anderen Varianten ist, die Euch bekannt sind. Dinge wie Korrektheit, Effizienz, etc.
std::string strip(std::string_view s) { /* Removes leading and trailing whitespace */ auto l = begin(s); auto r = end(s); while (l < r && std::isspace(*l)) ++l; while (r > begin(s) && std::isspace(*(r - 1))) --r; return { l, r }; }
-
Sieht okay aus und würde ich wahrscheinlich sehr ähnlich implementieren.
Allerdings macht es vielleicht Sinn auch wieder einenstd::string_view
zurückzugeben und den Anwender
entscheiden zu lassen, wann ein neuer String erstellt und die Zeichen kopiert werden sollen. Damit liesse
sich die Funktion auch elegant in einen Links- und einene Rechts-"Strip" zerstückeln (die kann man vielleicht
auch mal brauchen) - ohne zuzätzliche Kopien:std::string_view strip(std::string_view s) { return rstrip(lstrip(s)); }
-
while (r > begin(s) && std::isspace(*(r - 1))) --r;
Das geht schief, wenn der String nur aus whitespace besteht.
while (r > l && std::isspace(*(r - 1)))
wäre ok. Überhaupt kann r <= l ohnehin nur in diesem Fall wahr werden, und das ist es dann bereits am Ende der ersten Schleife.
auto l = begin(s); auto r = end(s); do { if (l == r) return {}; } while (std::isspace(*l++)); while (std::isspace(*--r)) ; return { l, ++r };
wenn man die Anzahl der zu prüfenden Bedingungen minimieren möchte.
Finnegan schrieb:
Allerdings macht es vielleicht Sinn auch wieder einen
std::string_view
zurückzugeben und den Anwender
entscheiden zu lassen, wann ein neuer String erstellt und die Zeichen kopiert werden sollen.Das ist gefährlich, falls der übergebene String bereits temporäres Objekt ist. Ein Funktion, bei der aufgrund des Namens explizit klar ist, dass ein view zurückgegeben wird (strip_view), wäre in Ordnung. Wenn Effizienz eine Rolle spielt, könnte man evtl. noch eine Überladung, die string-rvalues nimmt, in Erwägung ziehen, um unnötige free store-Allokationen zu vermeiden.
-
Ach mist, stimmt... die Bedingung
r > begin(s)
war echt blöd. Hab jetzt auch verstanden, warum der l < r Test innerhalb der zweiten Schleife überflüssig ist (es reicht praktisch ein einziger Test).Ich meine man muss den l Iterator auch noch um -1 korrigieren, da l immer inkrementiert wird, auch wenn l kein whitespace war.
-
camper schrieb:
Das ist gefährlich, falls der übergebene String bereits temporäres Objekt ist. Ein Funktion, bei der aufgrund des Namens explizit klar ist, dass ein view zurückgegeben wird (strip_view), wäre in Ordnung. Wenn Effizienz eine Rolle spielt, könnte man evtl. noch eine Überladung, die string-rvalues nimmt, in Erwägung ziehen, um unnötige free store-Allokationen zu vermeiden.
Da ist was dran, aber das eigentlich gefährliche ist in meinen Augen eher
auto
zu verwenden und
zu glauben, dass das dann in exakt zu dem Typ aufgelöst wird, den ich an dieser Stelle erwarte:auto b = strip(a);
In der "klassischen" Form ist das nämlich überhaupt kein Problem:
std::string b = strip(a);
Ein ähnliches Problem hat man übrigens auch, wenn ein Proxy-Objekt zurückgegeben wird:
std::vector<bool> v{ true, false, true }; auto a = v[1];
Der Typ von
a
ist hier auch nicht wie erwartetbool
, sondern die Proxy-Klassestd::vector<bool>::reference
.
Ebenso problematisch ist die Verwendung vonauto
, wenn man es mit Expression Templates zu tun hat.Kleines Plädoyer gegen die übermässige Verwendung von
auto
:Ich glaube nicht, dass es die Aufgabe von Bibliotheks-Entwicklern ist, die Klassen so zu bauen, dass man das
auto
-Schlüsselwort wie einvar
in diversen schwach typisierten Sprachen verwenden kann. Die Anwender
sollten sich darüber im Klaren sein, dassauto
ein Werkzeug für starke Typisierung ist und stets den exakten
Typen des zugewiesenen Ausdrucks deduziert. Man bekommt nicht auf magische Weise immer genau den
Typen, den man an dieser Stelle erwartet.Wenn man einen bestimmten Typen mit bekannten Eigenschaften haben möchte (wie hier
std::string
, der z.B.
keine internen Referenzen auf andere Objekte hält), dann sollte man den besser einfach explizit hinschreiben,
anstatt überallauto
zu benutzen und zu hoffen, dass es schon das richtige tut.
-
Finnegan schrieb:
Ich glaube nicht, dass es die Aufgabe von Bibliotheks-Entwicklern ist, die Klassen so zu bauen, dass man das
auto
-Schlüsselwort wie einvar
in diversen schwach typisierten Sprachen verwenden kann.Ich widerspreche. Es gibt auch genügend Experten, die diese Ansicht teilen, siehe das Operator Dot Proposal oder das alternative "Smart References Through Delegation", in denen eindeutig Mechanismen für dieses Problem bereitgestellt werden, weil es sich um eines handelt, dass einer internen Optimierung der Bibliothek entspringt und deshalb auch entsprechend gekapselt werden sollte.
-
Arcoth schrieb:
Finnegan schrieb:
Ich glaube nicht, dass es die Aufgabe von Bibliotheks-Entwicklern ist, die Klassen so zu bauen, dass man das
auto
-Schlüsselwort wie einvar
in diversen schwach typisierten Sprachen verwenden kann.Ich widerspreche. Es gibt auch genügend Experten, die diese Ansicht teilen, siehe das Operator Dot Proposal oder das alternative "Smart References Through Delegation", in denen eindeutig Mechanismen für dieses Problem bereitgestellt werden, weil es sich um eines handelt, dass einer internen Optimierung der Bibliothek entspringt und deshalb auch entsprechend gekapselt werden sollte.
Nehmen wir mal das
strip()
um das es hier geht: Mit diesen Proposals liessen sich tatsächlich Typen stricken,
bei denen mit imauto
-Kontext einstd::string
deduziert wird, und wenn ich das Ergebnis einemstd::string_view
zuweise, dieser ohne dass ein neuer String erstellt wird direkt in meiner Zielvariable landet? Ich habe die Proposals
nur überflogen und mich leider etwas schwer getan die Verbindung zu dem hier angesprochenen Problem zu ziehen.Wenn sowas möglich sein sollte, dann schlage ich mich gerne auch auf deine Seite. Mir ist wichtig, dass die vorhandenen
Optimierungen nicht nur intern bleiben, sondern bei Bedarf auch nach außen getragen werden können. Der "Default" für
eineauto
-Deduktion darf gerne der weniger fehleranfällige Typ wiestd::string
sein. Dennoch hätte ich schon gerne nur
eine Funktion (übersichtliche API) und möchte ich dem Anwender die Möglichkeit geben, z.B. einen flotten Parser zu
implementieren, der eben nicht bei jeder Gelegenheit neue Strings erstellt und diese etliche Male herumkopiert, sondern
lediglich mit String Views Indizies in den ursprünglichen Quellcode-String sammelt.Ich habe auch schonmal mit Expression Templates für eigene Matrix/Vektor-Klassen herumgespielt. Dort hat mich das
auto
-
Problem zugegebenermaßen auch schon gestört. Intuitiv ist das Ergebnis einer Matrixmultiplikation nämlich wiederum eine
Matrix und keine mysteriöseMatrixExpression
, die eigentlich nur eine interne Datenstruktur bleiben sollte. Ich habe seinerzeit
eine ganze Weile herumexperimentiert, um mit irgendwelchen Tricks herauszufinden, ob die MatrixExpression in einem Ausdrucks-
(ex1 * ex2) oder einem Zuweisungs-Kontext (auto a = ex1;) auftritt, und ob man den Typen, der von auto deduziert wird, dann
irgendwie beeinflussen kann - leider ohne Erfolg.So bin ich dann zu meiner aktuellen Ansicht gekommen, da mir die Optimierungen wichtiger sind, als die intuitive Verwendung von
auto
(sonst würde ich die Probleme auch in Python statt in C++ lösen). Sobald es dafür jedoch eine gute Lösung gibt, bin ich gerne
bereit meine Meinung wieder zu ändern - ich sähe es auch gerne wenn C++ für simpel und geradlinig gelöste Probleme weniger
Fallstricke bereithielte, ohne dabei "unter der Haube" an Mächtigkeit einzubüßen
-
Da wird natürlich jeder nach eigenen Präferenzen sein eigens Ding machen, bis der C++ Standard sich mal darum kümmert. Ich gebe mich mit folgender Version zufrieden, sofern ich nicht wieder einen Fehler eingebaut hab.
constexpr std::string_view trim_left_view(std::string_view s) { std::size_t l{}; while (l < s.size() && is_space(s[l])) ++l; return s.substr(l, s.size() - l); } constexpr std::string_view trim_right_view(std::string_view s) { std::size_t r{s.size()}; while (r && is_space(s[r - 1])) --r; return s.substr(0, r); } constexpr std::string_view trim_view(std::string_view s) { return trim_left_view(trim_right_view(s)); } inline std::string trim_left(std::string_view s) { return std::string{ trim_left_view(s) }; } inline std::string trim_right(std::string_view s) { return std::string{ trim_right_view(s) }; } inline std::string trim(std::string_view s) { return std::string{ trim_view(s) }; }
P.S.: Wenn ich das richtig verstanden habe, geht die string_view return Variante nut in die Hose, wenn man etwas wie
auto x = trim(string("abc"));
macht, stimmt das? Also mit diesem "temporären Objekt". Weilauto x = trim("abc");
ist ja kein Problem oder? Wie schafft man das überhaupt an trim ein temporäres Objekt zu übergeben, wo es auch Sinn machen könnte?
-
Warum das Rad doppelt erfinden, statt einen
reverse_iterator
einzusetzen, der sowieso vollständig wegoptimiert wird?
-
Arcoth schrieb:
Warum das Rad doppelt erfinden, statt einen
reverse_iterator
einzusetzen, der sowieso vollständig wegoptimiert wird?Wie würde man es verbessern mit reverse_iterator? Ich konnte irgendwie keinen string_view konstruieren aus Iteratoren, deshalb bin ich auf indices umgestiegen.