ZFX + Developia

Verfasst: **29.01.2023, 11:31**

Ich bin auf das Thema "McCabe-Metrik" gestossen (https://de.wikipedia.org/wiki/McCabe-Metrik) und mich würde interessieren, wie ihr dieses Thema beurteilt.

Wikipedia führt zwei Beispiele auf, bei denen Ersteres die höhere Komplexität in der McCabe-Metrik aufweist.
Beispiel 1:

Code: Alles auswählen

const String wochentagsname(const int nummer)
{
  switch (nummer)
  {
    case 1: return "Montag";
    case 2: return "Dienstag";
    case 3: return "Mittwoch";
    case 4: return "Donnerstag";
    case 5: return "Freitag";
    case 6: return "Samstag";
    case 7: return "Sonntag";
  }
  return "(unbekannter Wochentag)";
}

Beispiel 2:

Code: Alles auswählen

const String wochentagsname(const int nummer)
{
  string[] tage = new string[]
  {
    "Montag",
    "Dienstag",
    "Mittwoch",
    "Donnerstag",
    "Freitag",
    "Samstag",
    "Sonntag"
  };

  String ergebnis = "(unbekannter Wochentag)";
  if ((nummer >= 1) && (nummer <= sizeof (tage)))
  {
    ergebnis = tage[nummer - 1];
  }
  return ergebnis;
}

Gefühlt verhält es sich bei mir jedoch genau anders rum. Beim zweiten Beispiel habe ich eher das Gefühl, dass ich "in der Hitze des Gefechtes" vielleicht etwas falsch interpretieren könnte. Wohin gegen die Funktionsweise des ersten Beispiels für mich schneller und intuitiver klar wird.
Wie siehts bei euch aus?

Andere Frage am Rande:
Was ist das eigentlich für eine Sprache? Ist das Java?
Müsste der sizeof Operator in diesem Fall nicht den Wert 56 liefern? Es sind ja 7 Pointer a jeweils 8 Bytes (auf einem 64 Bit OS). Und müsste "String" in Java nicht konsequent gross geschrieben werden?

LG, starcow

Verfasst: **29.01.2023, 12:01**

Yep. Definitiv Beispiel 1 weit über Beispiel 2. Es ist viel klarer, was es tut. Zyklomatische Komplexität ist halt nur ein "naja, du hast halt viel Branching", sagt aber als rein mathematische Metrik wenig über die tatsächliche Qualität des Codes auf.

Darauf zu optimieren, dass eine solche Metrik niedrig ist, halte ich für ein fieses Anti-Pattern, was viel mehr schlechten Code erzeugt hat als guten.

Verfasst: **29.01.2023, 12:21**

Genau was xq sagt, alleine ist McCabe wenig aussagekräftig.
Deshalb berechnen die meisten Tools auch mehr als eine Metrik, z.B. neben der zyklomatischen Komplexität (McCabe) auch die kognitive Komplexität.
Generell bringt das alles aber nur was, um die Entwicklung in einem Projekt über mehrere Monate/Jahre im Blick zu behalten wenn viele Leute daran arbeiten, damit es nicht völlig aus dem Ruder läuft.

Verfasst: **29.01.2023, 12:27**

… wäre Beispiel 2 nicht so viel effizienter …

Verfasst: **29.01.2023, 13:05**

Das Beispiel ist zu sehr ein Spielzeug-Beispiel um irgendetwas Allgemeingültiges zu sagen. Um nur die ersten beiden offensichtlichen Fragen zu stellen: Was wenn es nicht Wochentage sind, sondern eine erweiterbare Liste? Will ich die Liste zur Laufzeit ändern können - "ändern" kann auch heißen aus einem config file lesen?

Wenn die zweite Antwort "nein" lautet, ist die korrekte Lösung in Java mMn keins von beidem sondern ein enum.

Verfasst: **29.01.2023, 13:08**

Also bei mir lief ersteres auf den ersten Blick auch wesentlich schneller:
https://quick-bench.com/q/bsfavhGm-f-L_OnayyjPjtX4zdo

Vielleicht habe ich irgendwo einen Fehler oder suboptimalen Code.
(vielleicht mal drüber schauen, habt ihr noch Optimierungs-Ideen?)

Verfasst: **29.01.2023, 13:26**

Es ist oft ein guter Indikator Gruselstellen zu finden.
Sonar's Cognitive Complexity https://www.sonarsource.com/resources/c ... omplexity/ finde ich besser als Metrik, aber auch nervig, weil die Berechnung nicht ganz so leicht zu merken ist.

Verfasst: **29.01.2023, 13:54**

NytroX hat geschrieben: ↑29.01.2023, 13:08Also bei mir lief ersteres auf den ersten Blick auch wesentlich schneller:
https://quick-bench.com/q/bsfavhGm-f-L_OnayyjPjtX4zdo

Vielleicht habe ich irgendwo einen Fehler oder suboptimalen Code.
(vielleicht mal drüber schauen, habt ihr noch Optimierungs-Ideen?)

Deine zweite Variante erzeugt zwei Strings – einmal "(unbekannter Wochentag)" und dann das tatsächliche Ergebnis. Korrigiert man das, ist der Unterschied 3-fach statt 8-fach.
Die zweite Implementierung erzeugt einmal einen std::vector mit fertigen std::strings drin, während die erste Implementierung jedes Mal einen neues String aus den Literals erzeugt. Das ist ein unfairer Vorteil für die zweite Variante.
Ich bin mir recht sicher, dass die Konstruktion von std::string samt Speicherallokation das Benchmark dominiert. Aber das ist wahrscheinlich gar nicht so schlimm, weil man es in den meisten echten Anwendungsfällen genauso machen würde.
Zuletzt und am wichtigsten: Du testest nicht zufällige Zugriffe, sondern immer das selbe Muster – damit gibt es im ganzen Benchmark nur acht Branches in immer gleicher Reihenfolge, und die Branch Prediction der CPU freut sich natürlich :D

Danach ist die zweite Variante nur noch minimal langsamer:

Code: Alles auswählen

static std::string wochentagsname1(const int nummer)
{
  switch (nummer)
  {
    case 1: return "Montag";
    case 2: return "Dienstag";
    case 3: return "Mittwoch";
    case 4: return "Donnerstag";
    case 5: return "Freitag";
    case 6: return "Samstag";
    case 7: return "Sonntag";
  }
  return "(unbekannter Wochentag)";
}

static std::string wochentagsname2(const int nummer)
{
  static char const * const tage[]
  {
    "Montag",
    "Dienstag",
    "Mittwoch",
    "Donnerstag",
    "Freitag",
    "Samstag",
    "Sonntag"
  };

  if ((nummer >= 1) && (nummer <= std::size(tage)))
  {
    return tage[nummer - 1];
  }
  return "(unbekannter Wochentag)";
}

static void Wochentage1(benchmark::State& state) {
  for (auto _ : state) {
    std::string wo_lol = wochentagsname1(1 + rand() % 8);
    benchmark::DoNotOptimize(wo_lol);
  }
}
BENCHMARK(Wochentage1);

static void Wochentage2(benchmark::State& state) {
  for (auto _ : state) {
    std::string wo_lol = wochentagsname2(1 + rand() % 8);
    benchmark::DoNotOptimize(wo_lol);
  }
}
BENCHMARK(Wochentage2);

Das überrascht mich, aber kurzer Blick ins Disassembly zeigt, dass GCC acht verschiedene Konstruktoren von std::string erzeugt hat, die jeweils die Länge und Adresse des Literals hard-coded haben. Ist natürlich eine riesen Inflation des Binaries und nicht so dolle, wenn die Caches kalt sind, aber da muss ich durchaus anerkennen, dass es schneller ist.

Der Blick zeigt allerdings auch, dass die erste Variante nur deshalb schneller ist, weil GCC sie automatisch in die zweite umgewandelt hat! Ich schaue mal kurz, ob ich das in C++ ausgedrückt kriege.

Edit: Hier – das baut GCC aus dem switch:

Code: Alles auswählen

static std::string makeMO() {
    return "Montag";
}
static std::string makeDI() {
    return "Dienstag";
}
static std::string makeMI() {
    return "Mittwoch";
}
static std::string makeDO() {
    return "Donnerstag";
}
static std::string makeFR() {
    return "Freitag";
}
static std::string makeSA() {
    return "Samstag";
}
static std::string makeSO() {
    return "Sonntag";
}
static std::string makeUnknown() {
    return "Sonntag";
}
static std::string wochentagsname2(const int nummer)
{
  using TagConstructor = std::string ();
  static constexpr TagConstructor * tage[] = {
    makeMO, makeDI, makeMI, makeDO, makeFR, makeSA, makeSO, makeUnknown
  };

  if ((nummer >= 1) && (nummer <= 7))
  {
    return tage[nummer - 1]();
  }
  return tage[7]();
}

Im Benchmark ist das sogar minimal schneller. Sehr interessant – die Optimierung kannte ich von noch nicht. Toll, dass das mittlerweile geht.

switch ist nur schnell, weil es intern auf die Tabellenversion umgestellt wird. Wenn das heutzutage möglich ist, kann man natürlich ohne Performance-Bedenken die switch-Variante bevorzugen.

Verfasst: **29.01.2023, 21:34**

Hmja, ich denke das ist generell ein Problem mit solchen Microbenchmarks - es ist halt nicht unbedingt realistisch.
Besser wäre natürlich das im Zusammenhang zu profilen, und nicht einfach mit ein paar Aufrufen wie in meinem Beispiel :-)

Mit deinen Anmerkungen hast du natürlich recht, insbesondere dass die Allokationen nicht gerade optimiert sind.
Aber: Ich habe mal einfach komplett naiv das Beispiel genommen - ist halt die Frage in wie weit man realistisch alles optimiert in einem echten Projekt. Beide Varianten sind mehr oder weniger "unoptimiert" - und da ist der verständlichere Code auch noch default-mäßig schnell.

Aber gut zu wissen, dass sich die beiden Varianten nicht signifikant in der Performance unterscheiden.
Man kann also den Code schreiben, der verständlicher ist - das ist gut.

Danke fürs Anschauen :-)

Verfasst: **01.02.2023, 10:37**

Sehr interessant, danke fürs Testen! :-)
Die Sprache scheint ja Java zu sein... Aber müsste hier sizeof nicht auch den Wert 56 liefern - wie unter C(++)? Und darf man in Java tatsächlich string wahlweise auch gross schreiben?
LG starcow

Verfasst: **01.02.2023, 10:55**

starcow hat geschrieben: ↑01.02.2023, 10:37 Sehr interessant, danke fürs Testen! :-)
Die Sprache scheint ja Java zu sein... Aber müsste hier sizeof nicht auch den Wert 56 liefern - wie unter C(++)? Und darf man in Java tatsächlich string wahlweise auch gross schreiben?
LG starcow

Ich denke nicht, dass es Java ist.

Verfasst: **01.02.2023, 13:50**

In C(++) müsste ja der Index Operator für ein Array zwingend NACH dem Identifier kommen.
Also z. B.
string array[3];
und nicht
string[3] array;

Verfasst: **01.02.2023, 19:28**

Es ist höchstwahrscheinlich nur Pseudocode.
Java hat kein "const". (bzw hat es als reserved keyword ohne Funktionalität).
In Wikipedia wurde Syntax-Highlighting von C verwendet, aber C scheint es ja definitiv auch nicht zu sein. :-)

Verfasst: **02.02.2023, 23:23**

An diese Möglichkeit hatte ich gar nicht gedacht! :-)

ZFX + Developia

McCabe-Metrik

McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik

Re: McCabe-Metrik