Code Formatierung



  • "hello world! ^^

    ich habe uns ein kleines Werkzeug gebastelt, das einen beliebigen Quelltext ein wenig tiefer legt, sprich: verschönert/strukturiert und an den bestimmten Stellen automatisch leere Kommentare einfügt.

    https://a.fsdn.com/con/app/proj/formater/screenshots/SnapShot.png

    https://sourceforge.net/projects/formater/?source=directory

    symtia



  • Dieser Thread wurde von Moderator/in Shade Of Mine aus dem Forum Webzeugs in das Forum Projekte verschoben.

    Im Zweifelsfall bitte auch folgende Hinweise beachten:
    C/C++ Forum :: FAQ - Sonstiges :: Wohin mit meiner Frage?

    Dieses Posting wurde automatisch erzeugt.



  • Ehrlich gesagt halte ich nicht viel von der Idee. Das heißt als Übungsaufgabe ist es sicher nicht verkehrt, aber es wird schon seinen Grund haben, dass du das Werkzeug nicht auf deinem eigenen Code angewendet hast, oder? Vertical Alignment ist im Allgemeinen nicht sehr beliebt und die Kommentare empfinde ich als sehr kontraproduktiv, weil ich nur noch wenige Zeilen Code auf einmal sehen kann.

    Aber gut, schauen wir uns mal den Code an. Da ich nicht über 1000 Zeilen kommentieren möchte, fange ich hinten an.

    int main(int argc, char* argv[]) {
    
        string strFileIn  = "c:\\temp\\in.txt"; 
        string strFileOut = "c:\\temp\\out.txt";
        string strFileLog = "c:\\temp\\log.txt";
    
        TBeauty* pBeauty = new TBeauty();   // Warum new? Besser: TBeauty beauty;
    
        pBeauty->Logger.Open          (strFileLog           );  // Aufgabe des Konstruktors
        pBeauty->ExecuteFiles         (strFileIn, strFileOut);
        pBeauty->Logger.Close         (                     );  // Aufgabe des Destruktors
    
    	delete pBeauty;    // unnötig, wenn du auf new verzichtest
    
        return 0;
    }
    

    Mal schnell einen Blick auf ein paar Funktionen in der Mitte:

    //........................................................................................
    // Fügt Leerzeichen in eine Zeile ein, ab iPos und setzt iPos auf die neue Position
    //........................................................................................
    bool TBeauty::Insert(string& str, int& iPos, int iSpaceCount) {
    
        // printf("insert: iPos:%3d iSpaceCount: %3d str: %s", iPos, iSpaceCount, str.c_str()); 
    
        string sSpace;
        for (int i=0;i<iSpaceCount;i++)
            sSpace += ' ';
    
        string sLeft  = str.substr(0   , iPos     );
        string sRight = str.substr(iPos, str.size());
    
        str   = sLeft + sSpace + sRight;
        iPos += iSpaceCount;
    
        return true;
    }
    

    Tut genau dasselbe wie:

    bool TBeauty::Insert(string& str, int& iPos, int iSpaceCount) {
        str.insert(iPos, iSpaceCount, ' ');
        iPos += iSpaceCount;
    
        return true;
    }
    

    Warum gibst du einen bool zurück? Warum benutzt du ungarische Notation? Warum steht im Funktionsnamen nicht, was eingefügt wird? Warum ist die Funktion Member von TBeauty? Besser:

    int insertSpaces(string& str, size_t pos, size_t spaceCount) {
        str.insert(pos, spaceCount, ' ');
        return pos + spaceCount;
    }
    

    Nächste Funktion:

    //........................................................................................
    // Prüft, ob Zeile nur aus SpacLeerzeichen besteht
    //........................................................................................
    bool TBeauty::HasSpaces(string& str) {
    
        for (int i=0; i < str.size(); i++)
            if (str[i] != ' ')
                return false;
    
        return true;
    }
    

    Irreführender Funktionsname. Ich würde erwarten, dass geprüft wird, ob str ein Leerzeichen enthält. Warum ist die Funktion Member von TBeauty? Auch hier gibt es wieder passende Memberfunktionen von std::string:

    bool isAllSpaces(const string& str) {
        return str.find_first_not_of(' ') == string::npos;
    }
    

    Nächste Funktion:

    //........................................................................................
    // Berechnet Leerzeichen für ein Tab
    //........................................................................................
    int TBeauty::Rest(int i1, bool bLastTab) {
    
        if (bLastTab)
            return iTabWidth;
    
        int iTabs = iTabWidth;
        while(iTabs < i1)
            iTabs += iTabWidth;
    
        int iDiff    = iTabs - i1;
        return iDiff == 0 ? iTabWidth : iDiff;
    }
    

    Wenn das vorherige Zeichen ein Tab war, ist i1 ein Vielfaches von iTabWidth, d.h. iDiff ist 0. Also ist das if überflüssig. Im mittleren Teil rundest du i1 auf das nächstgrößere Vielfache von iTabWidth auf. Dann nimmst du die Differenz von diesem Vielfachen und i1. Das riecht verdächtig nach einem Einsatzfall für den Modulo-Operator:

    int TBeauty::Rest(int i1, bool bLastTab) {
        return iTabWidth - (i1 % iTabWidth);
    }
    

    Achja, der Funktionsname ist ziemlich nichtssagend. Den Parameter bLastTab brauchen wir jetzt gar nicht mehr. Also:

    int neededSpacesForTabReplacement(int pos) {
        return iTabWidth - (pos % iTabWidth);
    }
    

    Nächste Funktion:

    //........................................................................................
    // Ersetzt Tabs in Leerzeichen
    //........................................................................................
    bool TBeauty::ReplaceTab(string& str) {
    
        string strResult;
        for (int i1=0; i1 < str.size(); i1++) {
            if (str[i1] == '\t') {
    
                bool bLastTab = true;
                if (i1 > 0)
                    bLastTab = str[i1-1] == '\t';
    
                int iRest = Rest(i1,bLastTab);
                string s  = "";
                for (int i=0;i<iRest;i++)
                    s += " ";
    
                strResult += s;
                continue;
            }
    
            strResult += str[i1];
        }
    
        str = strResult;
        return true;
    }
    

    Was sofort auffällt, ist, dass du in der Mitte Leerzeichen anhängst. Dafür haben wir aber doch gerade erst eine Funktion geschrieben. Ach nee, dafür müssten wir noch eine Variable für die aktuelle Position in strResult speichern. Dann lieber std::string::append benutzen. Außerdem hat Rest bzw. neededSpacesForTabReplacement jetzt nur noch einen Parameter. Also:

    bool TBeauty::ReplaceTab(string& str) {
        string strResult;
        for (int i1=0; i1 < str.size(); i1++) {
            if (str[i1] == '\t') {
                strResult.append(neededSpacesForTabReplacement(i1), ' ');
                continue;
            }
            strResult += str[i1];
        }
        str = strResult;
        return true;
    }
    

    Eigentlich ersetzen wir alle Tabs und nicht nur eins, d.h. der Funktionsname ist falsch. Außerdem verstehe ich nicht, warum wir true zurückgeben sollten. Besser:

    string TBeauty::ReplaceTabs(const string& str) {
        string strResult;
        for (int i1=0; i1 < str.size(); i1++) {
            if (str[i1] == '\t')
                strResult.append(neededSpacesForTabReplacement(i1), ' ');
            else
                strResult += str[i1];
        }
        return strResult;
    }
    

    Statt sich Zeichen für Zeichen eine Kopie von str zu erzeugen, könnte man doch eigentlich auch direkt in str die Tabs ersetzen:

    string TBeauty::ReplaceTabs(string str) {
        size_t i = 0;
        while((i = str.find('\t')) != string::npos)
            str.replace(i, 1, neededSpacesForTabReplacement(i), ' ');
        return str;
    }
    

    Tjoa, ich glaube, das reicht für den Anfang. Was du dir auf jeden Fall nochmal überlegen solltest: Was sollte in den Konstruktor verschoben werden? Warum benutzt du überall bool als Rückgabetyp? Welche Funktionen gehören in eine Klasse und welche sollten lieber als freie Funktionen definiert werden? Welche Funktionen bietet mir die Standardbibliothek? Ich hoffe, ich konnte dir ein paar Anregungen geben.



  • Lieber Michael ^^ diese kleine Fingerübung habe ich nicht für eine Doktorarbeit geschrieben. Ich habe mich auf Rügen während meines Urlaubs gelangweilt und dachte mir, da mein Projektleiter eindringlich darauf besteht, dass ich meinen Code so formatiere und denselben auch dokumentieren und ich deshalb jedesmal so einen Hals bekomme, dass ich diese langwierige und monotone Arbeit tun muss, erschuf ich einfach eine kleine funktionierende Lösung. Ehrlich gesagt ist es mir egal, wie ein Programm konstruiert ist. Wichtig ist, dass es funktioniert. Optimierungen sind natürlich wünschenswert, aber nicht notwendig.

    Betrachtet man dieses deiner Meinung nach bedeutungslose Programm aus einer anderen Perspektive, zeigt sich ihre wahre Bedeutung/Wert für meinen Arbeitgeber, meine Kollegen und mich: Es spart uns Stunden - und ich meine Stunden!!! - an Formatierungsarbeit und damit eine Freisetzung geistiger Energie für wichtigere Aufgaben.

    Deine konstruktiven Ideen nehme ich dankend entgegen, der Tonfall allerdings hätte etwas diplomatischer ausfallen können.

    Necip



  • Necip8 schrieb:

    da mein Projektleiter eindringlich darauf besteht, dass ich meinen Code so formatiere

    Mein Beileid. Dadurch hat das Programm natürlich seine Daseinsberechtigung.

    Deine konstruktiven Ideen nehme ich dankend entgegen, der Tonfall allerdings hätte etwas diplomatischer ausfallen können.

    Entschuldigung.



  • Beauty Beta 1.1.cpp upgraded!

    https://sourceforge.net/projects/formater/files/?



  • Hallo

    Vielleicht wäre das kleine und sehr nette Tool Artistic Style eine gute Alternative?

    Quelle:
    http://astyle.sourceforge.net/

    Gruß


Log in to reply