Vereinfachung möglich und sinnvoll?
-
@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
if ((uchar)*s > ' ') // most strings don't have any leading space, so handle this case as fast as possible
return (char *)s;early exit : Der String enthält keine führenden Leerzeichen, die weitere Verarbeitung kann sofort beendet werden
@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
while (*s && (uchar)*s <= ' ') // avoiding isspace() here, because it is much slower
s++;Iteration : Der char pointer wird so lange weitergesetzt, bis das Ende oder das erste Zeichen erreicht wurde, das kein Leerzeichen darstellt - dabei wird die Funktion
isspace()
vermieden und stattdessen händisch überprüft@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
return (char *)s;
regular exit : Der übriggebliebene String wird zurückgegeben (Pointer darauf)
-
@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
inline char *skipspace(const char *s)
Anzumerken ist hier vielleicht noch ... dass die Funktion mit der Signatur
(const char const *s)
nicht funktionieren würde, weil dann derchar
-Pointers
auch nicht modifiziert werden dürfte ...
-
@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
Das lässt sich doch bestimmt vereinfachen,
Wie es @SeppJ bereits gesagt hatte ... Deine Funktion tut insgesamt zu viel ... In einem ersten Schritt könntest Du diese in kleinere aufteilen, um dann zu sehen, was möglicherweise "wegkann".
Ich habe Deine Funktion nicht im Detail nachvollzogen.
-
@SeppJ
Zwei Fälle, da ein Plugin es anders darstellt. Möglich sind:"Eine Serie~Folge 1"
"Eine Serie ~ Folge 1"Beide Strings sollen in
"Eine Serie" und "Folge 1" gesplittet werden. Im OSD wird der zweite String in einer anderen Farbe angezeigt.Die Farrbe ist einstellbar und auch abschaltbar (if (ColorExtraTextFg))
-
@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
@SeppJ
Zwei Fälle, da ein Plugin es anders darstellt. Möglich sind:"Eine Serie~Folge 1"
"Eine Serie ~ Folge 1"Beide Strings sollen in
"Eine Serie" und "Folge 1" gesplittet werden. Im OSD wird der zweite String in einer anderen Farbe angezeigt.Die Farrbe ist einstellbar und auch abschaltbar (if (ColorExtraTextFg))
Wäre das nicht aus lösbar mit
- Splitte string mit trennzeichen '~'
- Trimme die strings von leerzeichen zu beginn/ende
Dann wäre das für beide fälle verwendbar ohne separaten code zu benötigen
-
Oder mit
regex
, das ginge auch, um beide zu berücksichtigen...
-
Ich habe das im Netz gefunden
// trim from left inline std::string <rim(std::string &s, const char* t = " \t\n\r\f\v") { s.erase(0, s.find_first_not_of(t)); return s; } // trim from right inline std::string& rtrim(std::string& s, const char* t = " \t\n\r\f\v") { s.erase(s.find_last_not_of(t) + 1); return s; }
Und der Code sieht jetzt so aus:
if (ColorExtraTextFg) { std::string tilde = Text; size_t found = tilde.find('~'); // Search for ~ if (found != std::string::npos) { std::string first = tilde.substr(0, found); std::string second = tilde.substr(found + 1, tilde.length()); rtrim(first); // Trim possible space on right side ltrim(second); // Trim possible space at begin Pixmap->DrawText(cPoint(0, 0), first.c_str(), ColorFg, ColorBg, Font); int l = Font->Width(first.c_str()); l += Font->Width('X'); Pixmap->DrawText(cPoint(l, 0), second.c_str(), ColorExtraTextFg, ColorBg, Font); } else // ~ not found Pixmap->DrawText(cPoint(0, 0), Text.c_str(), ColorFg, ColorBg, Font); } else { // No extracolor defined Pixmap->DrawText(cPoint(0, 0), Text.c_str(), ColorFg, ColorBg, Font); }
Schon viel kleiner...
Nur von cpplint kommt ne Warnung:Is this a non-const reference? If so, make const or use a pointer: std::string &s cpplint(warning:runtime/references)
Das kann ich nicht lösen
-
Schlechte Warnung, wo dir jemand seinen fragwürdigen, restriktiven Stil aufzwingen will. Ja, es ist mit Absicht eine non-const Referenz. Das kann schlechter Stil sein, wenn man es versteckt an unerwarteten Orten, aber hier ist das intuitiv klar und erwartet, dass die Funktion so benutzt werden sollte.
-
Warum wird eigentlich noch ein
return s;
benötigt? Und stimmt der Aufruf mit
ltrim(second);
Wird da nur die Referenz übergeben...
-
Nötig ist es nicht, aber es ermöglicht auch die Nutzung in verketteten Aufrufen, a la
cout << rtrim(my_string);
Wobei wir da tatsächlich so langsam in Gebiete kommen, wo die non-const Referenz zu gefährlichen Irrtümern führen kann. Beispielsweise wäre
result = ltrim(my_string)+ rtrim(my_string)
nicht so günstig.
-
@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
Nur von cpplint kommt ne Warnung:
Is this a non-const reference? If so, make const or use a pointer: std::string &s cpplint(warning:runtime/references)
Das kann ich nicht lösenVermutlich könnte der zweite Parameter auch so deklariert werden:
const char * const t
Aber das ist, wie es SeppJ erklärt hat, gewissermaßen unnötig erzwungen, würde ich sagen.
-
@SeppJ sagte in Vereinfachung möglich und sinnvoll?:
Nötig ist es nicht, aber es ermöglicht auch die Nutzung in verketteten Aufrufen, a la
cout << rtrim(my_string);
Wobei wir da tatsächlich so langsam in Gebiete kommen, wo die non-const Referenz zu gefährlichen Irrtümern führen kann. Beispielsweise wäre
result = ltrim(my_string)+ rtrim(my_string)
nicht so günstig.
Persönlich finde ich das besser entweder
std::string trim (const std::string& input);
oder
void trim (std::string& input_output);
zu wählen.
Ich vermisse mittlerweile in C++ ein Fortran Feature – die intent Deklaration von Parametern in Funktionen und Subroutinen. Funktionen können übrigens nur „in“ Parameter haben, Subroutinen „in“, „out“ und „in out“.
-
Ja, ich kann durchaus die Meldung des Linters da verstehen. Komplett neue Programmierer können sich da durchaus auf die Schnauze legen, und ein bösartiger Programmierer könnte absichtlich auch erfahrenen Programmierern ein Bein stellen. Aber ich finde es stark übertrieben. Die Hürde ist jetzt echt nicht hoch, das zu lernen. Und wenn du bösartige Feinde hast, werden die dich auf andere Weise kriegen. Ein bisschen Niveau muss man voraussetzen dürfen, sonst kommt man zu nix. Dieser Linter würde die gleiche Diagnose für einen großen Teil der Standardbibliothek liefern müssen.
-
Die boost-Stringbibliotheken bennenen ihre trim() Funktion explizit, da gibt es Sachen wie
trim( std::string& s )
undtrim_copy( std::string const& s )
, in allen Kombinationen (left
,right
mit und ohnecopy
im Namen). Sieht manchmal etwas sperrig aus, aber dafür sieht man beistd::string const cleaned = trim_copy( input ); oder std::string const cleaned2 = trim_left_copy( input );
aber auch sofort, was da passiert.
-
Dieses ganze
trim
,find
undsubstr
sieht mir so aus, als ginge es im Prinzip nur darum, den "richtigen" Substring in einem vorhandenen String zu finden. Da kann man eventuell auch überlegen, mitstd::string_view
zu arbeiten. So ließen sich einige Kopier- und Speicherallokations-Operationen einsparen... aber gut möglich, dass das bei diesem Code nur wenig ins Gewicht fällt und daher nicht so wichtig ist.Das sieht mir aber durchaus wie eine Rendering-Funktion aus, da lassen sich solche Optimierungen meistens schon rechtfertigen.
-
@DocShoe sagte in Vereinfachung möglich und sinnvoll?:
mit und ohne copy im Namen
Das gefällt mir. +1
Ich bin mittel-fortgeschrittener Anfänger, und für mich ist es eine Erleichterung, die Funktionsweise nicht über konstante bzw. nicht konstante Funktionsparameterdeklarationen "erahnen" zu müssen ... sondern sie direkt am Namen der Funktion ablesen zu können.
-
Ich habe das jetz so abgeändert, da ich das trim nur an den besagten Stellen brauche:
// Trim from left inline void ltrim(std::string &s, const char* t = " \t\n\r\f\v") { s.erase(0, s.find_first_not_of(t)); } // Trim from right inline void rtrim(std::string& s, const char* t = " \t\n\r\f\v") { s.erase(s.find_last_not_of(t) + 1); }
Es wäre auch ein Name im Stil von
inplace_rtrim()
denkbat
-
@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
find_first_not_of
Heißt das nicht
not_in
anstattnot_of
? In SQL heißt's so... Odernot_one_of
?@MegaV0lt sagte in Vereinfachung möglich und sinnvoll?:
Es wäre auch ein Name im Stil von
inplace_rtrim()
-
Und ob die Antwort richtig ist, verrät dir jetzt das Licht: std::basic_string<CharT,Traits,Allocator>::find_first_not_of
-
@Th69 sagte in Vereinfachung möglich und sinnvoll?:
Und ob die Antwort richtig ist, verrät dir jetzt das Licht: std::basic_string<CharT,Traits,Allocator>::find_first_not_of
If all characters in the range can be found in the given character sequence, npos will be returned.
Ich würde sagen ... die Namensgebung ist an der Stelle einfach nicht folgerichtig.