String einen substring über mehrere Zeilen

Sone

Jetzt müssten wir ein ganz großes Beispiel-File schreiben, und dann will ich wissen wessen schneller ist.

SeppJ

Sone schrieb:

Jetzt müssten wir ein ganz großes Beispiel-File schreiben, und dann will ich wissen wessen schneller ist.

Während deines noch compiliert hat meines schon mehrere Gigabyte geparsed.

Sone

SeppJ schrieb:

Sone schrieb:

Jetzt müssten wir ein ganz großes Beispiel-File schreiben, und dann will ich wissen wessen schneller ist.

Während deines noch compiliert hat meines schon mehrere Gigabyte geparsed.

Ko-kompiliert?

Und als es gelaufen ist?

Sone

Gut, was ist, wenn bei mir der Wert-Typ ein String ist und nicht unsigned ? Das beschleunigt es eventuell nochmal.
Außerdem kann checked_getline nochmal stark beschleunigt werden, in dem man nur am Anfang sentry erstellt und dann per Streambuf einliest.

SeppJ

Ich glaube, Performance ist hier ziemlich irrelevant. Wenn ich auf den typischen Anwendungsfall optimieren würde (wenige Einträge, wenige Kategorien) würde ich wohl sogar vector statt map nehmen, aber ich wollte dieses pädagogische Beispiel(!) weder unnötig verkomplizieren, noch habe ich hier vor, hier irgendwelche Programmierwettbewerbe an solch einem unsinnigen Beispiel auszutragen. Beim Parsen dürften sich unsere Programme unmerkbar wenig tun, weil die Hauptarbeit letztendlich beim Verarbeiten des Eingabestreams liegt, die bei beiden gleich ist. Ich würde jedoch spontan auf meine Implementierung setzen, was die kleinen Unterschiede angeht. Sie ist einfach unkomplizierter, das ist erfahrungsgemäß deutlich schneller.

SeppJ schrieb:

Ich würde jedoch spontan auf meine Implementierung setzen, was die kleinen Unterschiede angeht. Sie ist einfach unkomplizierter, das ist erfahrungsgemäß deutlich schneller.

Eine Verschachtelungstiefe von 8(!) Braces würde ich nicht unbedingt als unkompliziert bezeichnen.

Zudem:

SeppJ schrieb:

Da habe ich mir dann Mühe gegeben, wirklich alle Eventualitäten zu berücksichtigen und zu jeder eine aussagekräftige Meldung zu geben.

Deine Implementierung akzeptiert

[]
global=local
=
[/]

Klar, ist leicht gefixt, aber die Tatsache, dass dir so ein Fehler passiert, zeigt, dass dein Ansatz nicht wirklich einfach und robust ist.

Und schnell? Nicht wirklich. Jede Zeile wird 3 mal kopiert. Ich behaupte mal, die gleiche Implementierung in Python ist schneller, weil dort gesliced wird. Prägnant und schnell? Fehlanzeige in idiomatischem SeppJ-C++-Stil.

throw std::logic_error("Error parsing ini-file: Entry without a '='");

Exceptions -- lol? Ich zitiere mal die Doku:

<a href= schrieb:

std::logic_error ">It reports errors that are a consequence of faulty logic within the program such as violating logical preconditions or class invariants and may be preventable.

Jetzt erklär mir mal wie fehlerhafte Ini-Files preventable sind. Und falls sie das sind brauchst du keine sprechenden Fehlermeldungen.

Spätestens bei

catch (std::logic_error)

läuten die Alarmglocken. Seriously?

SeppJ schrieb:

ich wollte dieses pädagogische Beispiel(!) weder unnötig verkomplizieren

Fazit: Pädagogisch absolut schädlich.

Es gibt grundsätzlich zwei Ansätze, wie hier vorgegangen werden könnte. Der eine ist auf Geschwindigkeit optimieren. Das bedeutet in letzter Instanz mit einem Streambuf rumzuspielen und die chars direkt in den endgültigen String einzulesen. Der andere Ansatz ist auf Geschwindigkeit zu pfeifen und eine kurze und korrekte Implementierung zu schreiben.

(Man beachte, dass der Ansatz von Sone dem schlechtesten aus beiden Ansätzen entspricht, performancemässig die Hölle, weil >>ws viel kostet (sentry+ctype holen), widen viel kostet (ctype holen) und das verdammt häufig aufgerufen wird, jemand mit Ahnung hätte das gecached. Und der Code-Blah ist einfach überwältigend.)

Hier meine Implementierung:

typedef std::map<std::string, std::map<std::string, std::string> > ini_file;

std::istream& parse_ini(std::istream& in, ini_file& ini, std::string* err = 0)
{
  const boost::regex ini_line("\\s*([^\\#]*?)\\s*(\\#.*)?");
  const boost::regex start_cat("\[\\s*([^[]+?)\\s*\]");
  const boost::regex end_cat("\[/\\s*([^[]+?)\\s*\]");
  const boost::regex key_val("([^=]+?)\\s*=\\s*(.*?)\\s*");

#define FAIL_PARSE_IF(condition, msg)           \
  if (condition) {                              \
    if (err) *err = msg;                        \
    in.setstate(std::ios::failbit);             \
    return in;                                  \
  }

  std::string category;
  for (std::string l; getline(in, l);) {
    boost::smatch what;
    boost::regex_match(l, what, ini_line);
    l = what[1];

    if (boost::regex_match(l, what, end_cat)) {
      FAIL_PARSE_IF(what[1] != category, "closing wrong category, should be " + category);
      category.clear();
    } else if (boost::regex_match(l, what, start_cat)) {
      FAIL_PARSE_IF(!category.empty(), "nested categories not supported");
      category = what[1];
    } else if (boost::regex_match(l, what, key_val)) {
      ini[category][what[1]] = what[2];
    } else {
      FAIL_PARSE_IF(!l.empty(), "not a valid line: " + l);
    }
  }
  FAIL_PARSE_IF(!category.empty(), "category still open: " + category);

  return in;
#undef FAIL_PARSE_IF
}

Kurz und übersichtlich. Wird mit entsprechendem C++11-Support noch besser (s/boost/std/g + Raw-String literals)

Ich behaupte sogar, performancemässig ist das gleichauf mit SeppJ und Sone, weil ich nicht in Performance-Fallen getreten bin.

SeppJ

kritticker schrieb:

Zudem:

SeppJ schrieb:

Da habe ich mir dann Mühe gegeben, wirklich alle Eventualitäten zu berücksichtigen und zu jeder eine aussagekräftige Meldung zu geben.

Deine Implementierung akzeptiert
[]
global=local
=
[/]
Klar, ist leicht gefixt, aber die Tatsache, dass dir so ein Fehler passiert, zeigt, dass dein Ansatz nicht wirklich einfach und robust ist.

Das ist Absicht. Wieso sollten keine leeren Bezeichner erlaubt sein?

Und schnell? Nicht wirklich. Jede Zeile wird 3 mal kopiert. Ich behaupte mal, die gleiche Implementierung in Python ist schneller, weil dort gesliced wird.

Sone will hier Geschwindigkeit machen. Wen interessiert Geschwindigkeit bei einem ini-Parser? Mich nicht.

throw std::logic_error("Error parsing ini-file: Entry without a '='");
Exceptions -- lol? Ich zitiere mal die Doku:

<a href= schrieb:

std::logic_error ">It reports errors that are a consequence of faulty logic within the program such as violating logical preconditions or class invariants and may be preventable.

Und?

Spätestens bei
catch (std::logic_error)
läuten die Alarmglocken. Seriously?

Ja.

Der andere Ansatz ist auf Geschwindigkeit zu pfeifen und eine kurze und korrekte Implementierung zu schreiben.

Dann mach mal kürzer, deine korrekte Implementierung.

Hier meine Implementierung:

Oh, super. #include<iniparserlibrary> hätte ich wohl auch gekonnt und wäre noch kürzer.

SeppJ schrieb:

Das ist Absicht. Wieso sollten keine leeren Bezeichner erlaubt sein?

Falls es wirklich gewollt ist, sollte die Sektion "[]" von der globalen Sektion verschieden sein. Wüsste aber ehrlich gesagt nicht, weshalb leere Sections und/oder Keys erlaubt sein sollten (values schon, aber das unterstütze ich ja).

Sone will hier Geschwindigkeit machen. Wen interessiert Geschwindigkeit bei einem ini-Parser? Mich nicht.

Für diese Einstellung ist mir dein Code viel zu low-level.

throw std::logic_error("Error parsing ini-file: Entry without a '='");
Exceptions -- lol? Ich zitiere mal die Doku:

<a href= schrieb:

std::logic_error ">It reports errors that are a consequence of faulty logic within the program such as violating logical preconditions or class invariants and may be preventable.
Und?

Es ist ganz sicher kein Fehler der Programmlogik, wenn das Ini-File fehlerhaft ist.

Oh, super. #include<iniparserlibrary> hätte ich wohl auch gekonnt und wäre noch kürzer.

<iniparserlibrary> wird es wohl nie in absehbarer Zeit in die Standardlibrary schaffen. Boost.Regex hat das schon (ich warte nur noch auf Unterstützung)

SeppJ

kritticker schrieb:

SeppJ schrieb:

Das ist Absicht. Wieso sollten keine leeren Bezeichner erlaubt sein?

Falls es wirklich gewollt ist, sollte die Sektion "[]" von der globalen Sektion verschieden sein. Wüsste aber ehrlich gesagt nicht, weshalb leere Sections und/oder Keys erlaubt sein sollten (values schon, aber das unterstütze ich ja).

Doch, das war ganz bewusst Absicht. Wenn dies anders gewünscht wird, dann kann man dies trivial einbauen, da ohnehin bekannt ist, ob man gerade in einer Section oder im globalen Bereich ist. Also ich fand diese Semantik gut. Da das Format nicht genau vorgegeben war, kann der Programmierer tun, was er für richtig hält.

Sone will hier Geschwindigkeit machen. Wen interessiert Geschwindigkeit bei einem ini-Parser? Mich nicht.

Für diese Einstellung ist mir dein Code viel zu low-level.

Es gibt aber keine Methoden auf höherem Level in C++03.

throw std::logic_error("Error parsing ini-file: Entry without a '='");
Exceptions -- lol? Ich zitiere mal die Doku:

<a href= schrieb:

std::logic_error ">It reports errors that are a consequence of faulty logic within the program such as violating logical preconditions or class invariants and may be preventable.
Und?
Es ist ganz sicher kein Fehler der Programmlogik, wenn das Ini-File fehlerhaft ist.

Und?

Oh, super. #include<iniparserlibrary> hätte ich wohl auch gekonnt und wäre noch kürzer.

<iniparserlibrary> wird es wohl nie in absehbarer Zeit in die Standardlibrary schaffen. Boost.Regex hat das schon (ich warte nur noch auf Unterstützung)

Der Code ist C++03 und es geht darum zu zeigen, wie man so etwas macht. Fertige Bibliotheken gibt es für das Problem haufenweise.

@kritticker
Nette Lösung gegen den Bloat der hier, wie leider üblich, sonst gepostet wurde.

std::for_each( std::begin(child_nodes), std::end(child_nodes), static_cast<void(*)(void*)>(operator delete) );

Mein persönlicher WTF des Tages.

Sone

CreativeLabs schrieb:

std::for_each( std::begin(child_nodes), std::end(child_nodes), static_cast<void(*)(void*)>(operator delete) );

Mein persönlicher WTF des Tages.

Ja, das ist wirklich Bullshit. Nicht wahr? Sone hat, anstatt einfach
for( auto ptr : child_nodes ) delete ptr;
zu schreiben, alles katastrophal gemacht. Dazu auch noch missachtet, dass möglicherweise der Operator überladen wurde...

Leutchen, das war eine unvollständige Zwischenversion (!).

Sone schrieb:

Leutchen, das war eine unvollständige Zwischenversion (!).

Will nur niemand lesen. Warum musst Du jedes deiner tollen Experimente auch hier veröffentlichen?

SeppJ schrieb:

kritticker schrieb:

Falls es wirklich gewollt ist, sollte die Sektion "[]" von der globalen Sektion verschieden sein.

Doch, das war ganz bewusst Absicht.

Na dann...

Es gibt aber keine Methoden auf höherem Level in C++03.

Seit wann darf hier nur Code in C++03, der ausschliesslich die Standardbibliothek benutzt gepostet werden? Boost steht explizit in der Forenbeschreibung, wenn es mit der reinen C++03 Standardbibliothek nicht vernünftig geht.

Es ist ganz sicher kein Fehler der Programmlogik, wenn das Ini-File fehlerhaft ist.

Und?

Du verwendest die Klasse völlig falsch. Selbst der C++-Standard hält die Nutzung hier für unangebracht.

The class logic_error defines the type of objects thrown as exceptions to report errors presumably detectable before the program executes, such as violations of logical preconditions or class invariants.

Du könntest genausogut throw (std::valarray*)0; schreiben.

Der Code ist C++03 und es geht darum zu zeigen, wie man so etwas macht. Fertige Bibliotheken gibt es für das Problem haufenweise.

"Machen" würde es so niemand.
Der OP war weder daran interessiert, noch wird er es verstehen (rtrim???)
Die Beschränkung auf C++03 ist willkürlich zumal der OP C++11 verwendet (std::string-Konstruktor für fstream)
Stilistisch ist das falscher (logic_error) und hässlicher (8 Einrückungsebenen) Code.

Naja, vielleicht hat der Code irgendeinen einen Mehrwert (ich seh ich ehrlich gesagt nicht). Aber der std::logic_error muss korrigiert werden.

SeppJ

kritticker schrieb:

Aber der std::logic_error muss korrigiert werden.

Nein. Wenn du das nicht verstehst, dann tust du mir leid. Aber von dem sonstigen Unsinn her nehme ich mal an, du willst bloß absichtlich rumscheißern.

SeppJ schrieb:

kritticker schrieb:

Aber der std::logic_error muss korrigiert werden.

Nein. Wenn du das nicht verstehst, dann tust du mir leid.

Wie soll ich dich verstehen, wenn du nur "Und?" und "Nein." schreibst? Ich begründe wenigstens.

SeppJ

Ich bin kein Sklave irgendeiner Beschreibung. logic_error hat vom Namen her einen passenden Flavour. Ich nehme ihn. Es ist ein Fehler in der Logik der Datei aufgetreten. Mir doch egal, dass es nicht durch einen internen Fehler geschehen ist (natürlich passiert so etwas nicht durch interne Fehler, meine Programme sind schließlich richtig). In der Doku schreibe ich, dass das Ding einen logic_error schmeißt, wenn die Operation aufgrund eines Fehlers im logischen Aufbau der Datei fehlschlägt. Das allgemeine ios::failure kann diesen Flavour nicht ausdrücken.

minastaros

@kritticker, SeppJ: Ihr solltet wirklich über die Sache streiten, nicht über Euch. Ich habe das Gefühl, dass Ihr einfach eine andere Sichtweise darüber habt, wie ein Fehler aus einer Ini-Datei (falsches Format etc.) zu klassifizieren ist.

kritticker sieht " logic_error "s explizit als solche an, die aus der Programmlogik kommen; die Ini-Datei dagegen sieht er auf der Ebene von Benutzereingaben, daher kann es (da das Programm selbst perfekt ist) per Definition von "logic_error" (errors presumably detectable before the program executes) kein solcher sein.

Prinzipiell sehe ich das ähnlich, würde in diesem Fall eher ein " runtime_error " verwenden oder eine eigene Klasse, davon abgeleitet, sowas in der Art "user_input_error" oder "bad_config_error"...

An std::logic_error ist technisch nichts auszusetzen, wer für sich allein programmiert, soll den ruhig verwenden, wie er will.

Sofern allerdings jemand anders den Code anschaut, sollte man sich an gewisse allgemein anerkannte Konventionen halten. Was im Standard steht ist allgemein anerkannt. Wenn der Standard unsinnig ist, soll es nicht verwendet werden.

SeppJ schrieb:

Mir doch egal, dass es nicht durch einen internen Fehler geschehen ist (natürlich passiert so etwas nicht durch interne Fehler, meine Programme sind schließlich richtig).

Es ist nur so, dass jeder andere Programmierer logic_error als internen Fehler ansieht. Die Klasse umzudeuten halte ich für nicht so klug. Wenn dir "logischer Fehler" gefällt, leite dir halt eine Klasse logic_input_error von std::runtime_error ab.

Ich persönlich meine, dass ios::failure genau dieses Flavour (fehlerhafter User-Input) ausdrückt. Die genaue Fehlernachricht steht für den, der sie wissen möchte in einem separaten Parameter.

minastaros

Noch ein paar allgemeine Fragen:

@ Helppls1234: Wie fix ist denn Dein Dateiformat? Das Ini-Format, was ich so kenne, braucht eigentlich keine schließenden Tags. Es enthält "Sections" (das in den eckigen Klammern) und dahinter Key-Value-Paare, bis zur nächsten Section oder Dateiende. Deine Datei wäre dann z.B.:

`[Schurke]

Str=12

Dex=17

Int=4

[Krieger]

Str=18

Dex=10

Int=2`

Das würde den Parser etwas vereinfachen...

@ SeppJ: Warum gibst Du bei den get -Methoden nicht einfach den Wert zurück? Schon klar, Template-Typ muss in die Signatur, aber irgendwie fühlt es sich komisch an, bei einem get() den Wert als Referenz zu übergeben und die Möglichkeit eines Rückgabewertes durch void zu verschenken.

Bei einer Ini-Datei würde ich aus dem Bauch raus die Values entweder fix als string zurückgeben (und die Interpretation nach int oder double später; der Parser soll ja den Typ der einzelnen Values gar nicht kennen müssen), oder gerne die Templates so wie sie sind und dann speziell einen anderen Typ anfordern.

Also statt:

int a;
sip.get_verbose(std::cout, "Krieger", "Str", a);

etwa so:

int a = sip.get_verbose<int>(std::cout, "Krieger", "Str");

@Sone: Warum nicht einfach ein vector aus shared_ptr ? Bei Containern aus puren Pointern (und den dazugehörigen selbstgeschriebenen Lösch-Schleifen) krieg ich inzwischen Gänsehaut). Also statt

std::vector<Node*> child_nodes; // Darf *nicht* verändert werden - Inhalt wird gelöscht

    ~Node()
    {
        std::for_each( std::begin(child_nodes), std::end(child_nodes), static_cast<void(*)(void*)>(operator delete) );
    }

std::vector<std::shared_ptr<Node>> child_nodes; // Darf *nicht* verändert werden - Inhalt wird gelöscht