unübersichtlicher Code :( [gelöst]



  • Hallo Gemeinschaft,

    ich muss in einem Programm die Zuordnung von Präfixen zu physikalischen Einheiten realisieren.
    ZB. habe ich einen Wert 123456789.01 W (Watt) als String und muss daraus 123,46 MW (MegaWatt) machen. Der Zahlenwert und die Einheit liegen getrennt vor, dh. ich kann den Zahlenwert-String testen und dann den Einheit-String und Zahlenwert-String getrennt modifizieren. Im Einheit-String habe ich einen Punkt als Platzhalter für das später einzufügende Präfix (Kilo, Mega, Milli etc.) vor die Einheit gesetzt.

    ReturnStrg= "     .W ";
    

    Ohne den Zahlenwert zu verändern sieht das Ganze jetzt so aus:

    if(ReturnStrg.Pos(".")>0)                     // wenn Platzhalter vorhanden:
       {
          float TmpFloat= atof(GetStrg.c_str());
          if(TmpFloat>999999.99)
          { ReturnStrg.Insert("M", ReturnStrg.Pos(".")); }
          else
          {
             if(TmpFloat>999.99)
             { ReturnStrg.Insert("k", ReturnStrg.Pos(".")); }
             else
             {
                if(TmpFloat>0.00)
                { ReturnStrg.Insert("   ", ReturnStrg.Pos(".")); }
                else
                // ....
             }
          }
          ReturnStrg.Delete(ReturnStrg.Pos("."), 1); // Platzhalter löschen
       }
       return(ReturnStrg);
    

    Dieser Code ordnet zwar nun die richtigen Präfixe zu, ich finde ihn aber ziemlich unübersichtlich und lang (oben habe ich die Hälfte rausgekürzt)... Leider zerbreche ich mir schon seit Stunden den Kopf, finde aber irgendwie keine kürzere, übersichtlichere Lösung. Deshalb meine Frage: Wie kann ich obigen Code kürzer / übersichtlicher gestalten?

    [Edit]

    if(ReturnStrg.Pos(".")>0)                             // wenn Platzhalter vorhanden:
       {
          float TmpFloat= atof(GetStrg.c_str());
          AnsiString PrfxStrg= "";
          if(TmpFloat>999999.99) PrfxStrg= "M";
          else
             if(TmpFloat>999.99) PrfxStrg= "k";
             else
                if(TmpFloat>0.00) PrfxStrg= "  ";
                else
                   // ....
          ReturnStrg.Insert(PrfxStrg, ReturnStrg.Pos("."));
          ReturnStrg.Delete(ReturnStrg.Pos("."), 1);         // Platzhalter löschen
       }
       return(ReturnStrg);
    

    soweit habe ich den Code nun schon gekürzt, aber es muss doch auch eleganter gehen, oder nicht? Meine ursprüngliche Frage steht nach wie vor.
    [/Edit]

    MfG



  • Falls Du mit unübersichtlich die if/else-Kaskade meinst:
    Hmh, ich würde mich mal mit dem dekadischen Logarithmus befassen, und dann eine konstante Lookup-Tabelle o.ä. vewenden. Ich weiß aber nicht, wie effektiv diese Funktion implementiert ist bzw. es für Dich ausreichend ist.
    Dann würde ich die Arbeit in zwei Teile trennen: Zuerst den Präfix ermitteln, dann den String zerlegen und neu zusammenbauen.



  • wie wärs mit (PSEUDO CODE GEFAHR :D):

    string array mit allen Präfixen("","K","M",...);
    int index = 0;
    while(TmpFloat > 0){
      TmpFloat /= 1000;
      index++;
    }
    
    string output_string = TmpFloat+Präfix_array[index]+"W";
    

    !?
    hoffe das hilft



  • Hab grad einfach mal drauf los getippt, weil ich es auch wie witte erwähnte zunächst mal mit nem LUT versuchen würde.
    Allerdings wird der Code dadurch vielleicht was kleiner, aber nicht gerade einfacher oder schneller 😃

    Nur mal so ohne wirklich zu testen:

    char const PrefixLUT[101] = {'y','z', 'a',...,' ' /*Nullpunkt*/,...,'k','M','G',...};
    
    float TmpFloat = atof(GetStrg.c_str());
    short int Sign = (TmpFloat < 0 ? -1 : 1);
    
    int PrefIndex = (int)(  51 + Sign*(ceil(log10(abs(TmpFloat)))) ); /*Horrible :) */
    PrfxStrg = PrefixLUT[PrefIndex];
    


  • witte schrieb:

    Falls Du mit unübersichtlich die if/else-Kaskade meinst:
    Hmh, ich würde mich mal mit dem dekadischen Logarithmus befassen, und dann eine konstante Lookup-Tabelle o.ä. vewenden.

    Das ist gut, super! So siehts aus (noch nicht ganz vollständig):

    if(ReturnStrg.Pos(".")>0)
       {
          AnsiString PrfxStrg[9]= {"  ","  ","  ","k","k","k","M","M","M"};
          int iTest= (int)log10(atof(GetStrg.c_str()));
          ReturnStrg.Insert(PrfxStrg[iTest], ReturnStrg.Pos("."));
          ReturnStrg.Delete(ReturnStrg.Pos("."), 1);
       }
       return(ReturnStrg);
    

    witte schrieb:

    Ich weiß aber nicht, wie effektiv diese Funktion implementiert ist bzw. es für Dich ausreichend ist.

    Was bedeutet das?

    [Edit]
    Und mit Berücksichtigung und Weiterentwicklung von majin's Idee:

    if(ReturnStrg.Pos(".")>0)
       {
          AnsiString PrfxStrg[3]= {"  ","k","M"};
          short PrfxIdx= (short)(log10(atof(GetStrg.c_str())) / 3);
          ReturnStrg.Insert(PrfxStrg[PrfxIdx], ReturnStrg.Pos("."));
          ReturnStrg.Delete(ReturnStrg.Pos("."), 1);
       }
       return(ReturnStrg);
    

    [/Edit]

    Danke schonmal für alle Antworten bis hierher! 👍 🙂



  • Kolumbus schrieb:

    witte schrieb:

    Ich weiß aber nicht, wie effektiv diese Funktion implementiert ist bzw. es für Dich ausreichend ist.

    Was bedeutet das?

    Ich meinte simpel die Rechengeschwindigkeit der Funktion. Man könnte ja auch die signifikanten Stellen vor/nach dem Komma ermitteln.



  • Kurze Anmerkung:

    Negative Werte scheinst du wohl nicht zu benötigen. Kannst du denn mit Sicherheit sagen, dass alle(!) eingehenden Werte positiv sind? Ansonsten würde ich zumindest ein abs() mit reinnehmen, weil log10() mit negativen Werten ein Exception wirft.



  • witte schrieb:

    Kolumbus schrieb:

    witte schrieb:

    Ich weiß aber nicht, wie effektiv diese Funktion implementiert ist bzw. es für Dich ausreichend ist.

    Was bedeutet das?

    Ich meinte simpel die Rechengeschwindigkeit der Funktion.

    Ist nicht so wild, wenn die etwas länger braucht... mit if-else-Kaskade wär's natürlich schneller.

    witte schrieb:

    Man könnte ja auch die signifikanten Stellen vor/nach dem Komma ermitteln.

    Auch 'ne Überlegung wert, Danke! 🙂

    Wäre das Folgende eigentlich unüblich (mal abgesehen von der Übersichtlichkeit), oder gibt es keine weitere Gründe die gegen Zeile 4 sprechen???

    if(ReturnStrg.Pos(".")>0)
       {
          AnsiString PrfxStrg[3]= {"  ","k","M"};
          ReturnStrg.Insert(PrfxStrg[(short)(log10(atof(GetStrg.c_str())) / 3);], ReturnStrg.Pos("."));
          ReturnStrg.Delete(ReturnStrg.Pos("."), 1);
       }
       return(ReturnStrg);
    

    __yoshi schrieb:

    Kurze Anmerkung:

    Negative Werte scheinst du wohl nicht zu benötigen. Kannst du denn mit Sicherheit sagen, dass alle(!) eingehenden Werte positiv sind?

    Ja, ich schneide das Minuszeichen vorher raus 😉

    __yoshi schrieb:

    Ansonsten würde ich zumindest ein abs() mit reinnehmen, weil log10() mit negativen Werten ein Exception wirft.

    Danke für den Hinweis 🙂



  • Ähm, sagt mir, wenn ich mich irre, aber ist nicht z.B.:
    log10(999.99) = 3.0 => 3.0 / 3 = 1.0 => short 1.0 = 1 => "k" ?!

    Edit:
    Schon gut, ich bin grade auf das Aufrunden der Nachkommastellen reingefallen ^^ Müsste mit einem double ja gehen. *räusper*



  • witte schrieb:

    Kolumbus schrieb:

    witte schrieb:

    Ich weiß aber nicht, wie effektiv diese Funktion implementiert ist bzw. es für Dich ausreichend ist.

    Was bedeutet das?

    Ich meinte simpel die Rechengeschwindigkeit der Funktion. Man könnte ja auch die signifikanten Stellen vor/nach dem Komma ermitteln.

    dann meinst du effizient, nicht effektiv.
    effektiv ist die funktion, wenn sie ihre aufgabe erfüllt. effizient ist sie, wenn sie es außerdem mit minimalem aufwand tut.



  • __yoshi schrieb:

    Ähm, sagt mir, wenn ich mich irre, aber ist nicht z.B.:
    log10(999.99) = 3.0 => 3.0 / 3 = 1.0 => short 1.0 = 1 => "k" ?!

    Edit:
    Schon gut, ich bin grade auf das Aufrunden der Nachkommastellen reingefallen ^^ Müsste mit einem double ja gehen. *räusper*

    Hätte von mir sein können der Beitrag ^^

    klugscheißer schrieb:

    witte schrieb:

    Kolumbus schrieb:

    witte schrieb:

    Ich weiß aber nicht, wie effektiv diese Funktion implementiert ist bzw. es für Dich ausreichend ist.

    Was bedeutet das?

    Ich meinte simpel die Rechengeschwindigkeit der Funktion. Man könnte ja auch die signifikanten Stellen vor/nach dem Komma ermitteln.

    dann meinst du effizient, nicht effektiv.
    effektiv ist die funktion, wenn sie ihre aufgabe erfüllt. effizient ist sie, wenn sie es außerdem mit minimalem aufwand tut.

    So ein Klugscheißer dieser klugscheißer ^^

    Wikipedia schreibt dazu:

    Ein dafür anschauliches Beispiel:

    „Eine Flasche Champagner auf eine umgestürzte Kerze zu gießen, ist effektiv, falls das Feuer danach gelöscht ist. Es ist auch effizient, falls die durch das Feuer verursachten Schäden die Kosten des Champagners übersteigen würden.“

    Natürlich wäre ich bemüht die Kerze nicht nur effektiv, sondern auch effizient zu löschen... Aber ich fürchte bis ich den Kassenbon vom Champagner gefunden und ausgerechnet habe ob ich die Kerze besser anderweitig lösche, steht die halbe Bude in Brand... 🤡


Anmelden zum Antworten