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 einen std::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 erwartet bool , sondern die Proxy-Klasse std::vector<bool>::reference .
    Ebenso problematisch ist die Verwendung von auto , 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 ein var in diversen schwach typisierten Sprachen verwenden kann. Die Anwender
    sollten sich darüber im Klaren sein, dass auto 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 überall auto 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 ein var 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 ein var 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 im auto -Kontext ein std::string deduziert wird, und wenn ich das Ergebnis einem std::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
    eine auto -Deduktion darf gerne der weniger fehleranfällige Typ wie std::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öse MatrixExpression , 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". Weil auto 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.