Stilfrage



  • Hi,
    wäre toll wenn sich jmd meinen Code anschauen würd und dazuschreibt was man wie besser machen sollte.
    Das Programm: man gibt ein Datum ein, wenn dazu ein Spieltag gefunden wird, wird ein Link zum Spielbericht gesetzt. Anschließend gibt man noch das Ergebnis ein.
    Das ganze wird dann wieder in die Textdatei geschrieben
    - Sollte ich die Variablen als Klassenvariablen deklarieren oder den Funktionen übergeben?
    - findeBegegnung() und suchen() als Methoden von Spielplan? formatDatum dafür nicht
    - das Array dynamisch? wie?

    #include <iostream>
    #include <fstream>
    #include <string>
    #include <sstream>
    
    using namespace std;
    
    class Spielplan {
      public:
              Spielplan();
              int suchen();
              string ersetzen(string, int);
              string formatDatum(string);
    
      private:
              string sDatum;
              string sArray[3]; //Datum und die beiden folgenden Zeilen speichern
              size_t found;
    
    };
    
    Spielplan::Spielplan() {
      bool bDatum = false;
      while (!bDatum)  
        {                   
            cout << "Datum eingeben" << endl;
            cin >> sDatum;
            //checken ob dd.mm.yyyy
            if (sDatum.length() == 10 && sDatum.compare(6,1,".") )  {bDatum = true; }
            else
            {
                if (sDatum.length() == 8 && sDatum.compare(6,1,".")) 
                   {  
                        sDatum.insert(6, "20",2);
                        bDatum = true;
                   }
                else
                        cout << "Fehlerhaftes Datum" << endl;
            }        
    
        }                     
    }
    
    string Spielplan::formatDatum(string datum) {
      //dd.mm.yyyy -> yymmdd
      string newdatum = "";
      newdatum.insert(0, datum, 8, 2);
      newdatum.insert(2, datum, 3, 2);
      newdatum.insert(4, datum, 0,2 ) ;   
      return newdatum;     
    }       
    
    void schreiben(string file, string inhalt) {
             ofstream output;
             output.open(file.c_str(), ios::app); //ans Ende hängen (app)
             output << inhalt << endl;
             //output.close();
    
    }
    
    void findeBegegnung(string zeile, int position) {
            size_t length;
            char begegnung[100];  //dynamisch machen
            length=zeile.copy(begegnung, zeile.length() - int(position) - 8, int(position)+3); //richtige Stelle finden
            begegnung[length] = '\0';
            cout << "Die Begegnung lautet: " << begegnung << endl;
    }
    
    int Spielplan::suchen() {
      const string datei =  "saison1011_1.txt";                    
      ifstream txtSpielplan;  
      txtSpielplan.open(datei.c_str(), ios::in);
      //Textfile zeilenweise durchsuchen und in sArray zwischenspeichern
      int gefunden = 0;
      while (!txtSpielplan.eof()) 
      {
             getline(txtSpielplan, sArray[0], '\n');
             //geänderte datei schreiben
             schreiben("output.txt", sArray[0]); //in Textfile schreiben
             //found ist Klassenvariable
             found=sArray[0].find(sDatum);
             if (found!=string::npos)
             {  
             gefunden = 1;
             //wenn das Datum gefunden wurde, dann lies die nächsteN BEIDEN Zeile ein, hier wird der Link gesetzt
                    for (int i=1; i<=2; i++) 
                    {   
                            getline(txtSpielplan, sArray[i], '\n');
                            found=sArray[i].find_last_of(";");
                            if (found!=string::npos)
                            {
                                    if (i==1) //1. Zeile = Begegnung, 2. Zeile = Ergebnis
                                    {         
                                            findeBegegnung(sArray[i], found); 
                                    }
    
                                    schreiben("output.txt", ersetzen(sArray[i], i));  
    
                            }   
                    } 
              }
              //nix gefunden, Zeile trotzdem schreiben
                  //output.close;            
      }          
      //saison1011_1 durch output ersetzen (Inhalt)
      txtSpielplan.close();
      remove(datei.c_str());
      rename("output.txt", datei.c_str());
      if (!gefunden) { cout << "Datum wurde nicht gefunden" << endl; }
      return 0;
    }
    
    string Spielplan::ersetzen(string Str, int i) {
    
      //link zusammenfügen
      const string htmlTag = "<a href=\"http://www.test12.de/m1/m1_" + formatDatum(sDatum) + ".html\">" ;
      Str.insert(found +3, htmlTag);
      if (i==2)
         {
              string sErgebnis;
              cout << "Ergebnis?" << endl;
              cin >> sErgebnis;
              Str.replace(Str.length() - 6, 1, sErgebnis + "</a>");
              return Str;
         }
      Str.insert(Str.length() - 5, "</a>");
      return Str;
    
    }
    
    int main() {
    
             Spielplan m1;
             m1.suchen();
    
     return 0;   
    }
    


  • string ersetzen(string, int);//Hier müssen Namen rein
    ...
              string sDatum;//UN ist out
    ...
              size_t found;//ist mir ganz unverständlich, wozu?
    


  • Das size_t found hab ich in Ahnlehnung an den Referenz-Code. Du meinst man könnte auch int nehmen?

    // string::find
    #include <iostream>
    #include <string>
    using namespace std;
    
    int main ()
    {
      string str ("There are two needles in this haystack with needles.");
      string str2 ("needle");
      size_t found;
    
      // different member versions of find in the same order as above:
      found=str.find(str2);
      if (found!=string::npos)
        cout << "first 'needle' found at: " << int(found) << endl;
    
      found=str.find("needles are small",found+1,6);
      if (found!=string::npos)
        cout << "second 'needle' found at: " << int(found) << endl;
    
      found=str.find("haystack");
      if (found!=string::npos)
        cout << "'haystack' also found at: " << int(found) << endl;
    
      found=str.find('.');
      if (found!=string::npos)
        cout << "Period found at: " << int(found) << endl;
    
      // let's replace the first needle:
      str.replace(str.find(str2),str2.length(),"preposition");
      cout << str << endl;
    
      return 0;
    }
    

    Was meinst du mit UN?



  • Diotor schrieb:

    Das size_t found hab ich in Ahnlehnung an den Referenz-Code. Du meinst man könnte auch int nehmen?

    size_t ist schon ok. Nur weiß ich nicht, wozu das Attribut überhaupt vorhanden ist.



  • UN = Ungarische Notation. Das sind die Präfixe wie sName für string usw.



  • Warum sollte ungarische notation out sein?



  • Ist halt so 😉

    In welchen (neuen) Software-Projekten wird sie denn noch verwendet?



  • In allen in die die Ungarische Notation für sinnvoll gehalten wird?


  • Administrator

    zeusosc schrieb:

    Warum sollte ungarische notation out sein?

    Weil sie keinen Mehrwert bringt, vor allem nicht in C++. In C++ interessiert dich der Typ der Variable gar nicht sondern nur die angebotene Schnittstelle. In C++ kann man die Typen beliebig austauschen und sie haben immer noch die gleiche Schnittstelle. Nach ungarischer Notation müsstest du jedesmal, wenn du den Typ austauschst, auch den Variablennamen ändern gehen, das ist ein unnötiger zusätzlicher Aufwand, welcher auch schnell mal vergessen wird, wodurch man inkonsistenten Code bekommt.
    Auch sollte man kurze Funktionen verwenden, keine globalen Variablen einsetzen, usw., wodurch man eigentlich immer im Kopf haben sollte, von welchem Typ die Variable war.
    Die Kürzel sind zudem für ungeübte Leser der UN nur schwer zu verarbeiten. Der Code wird dadurch also unleserlich.
    Desweiteren gibt es die eine UN von Microsoft, welche grundsätzlich eine falsche Interpretation der ursprünglichen UN ist. Die ursprüngliche UN wollte nicht den Datentyp beschreiben, sondern den Funktionstyp der Variable. Aber auch dies gilt als veraltet.
    In neueren Sprachen kommen keine UN Empfehlungen mehr. Auch Microsoft rät z.B. in C# davon ab.

    Statt die UN zu verwenden, sollte man eher Variablennamen verwenden, welche die Variable genau beschreibt. Also keine Abkürzungen oder nichts aussagende Namen verwenden. Das hilft deutlich mehr beim Lesen des Codes.

    Gibt übrigens zahllose Diskussionen darüber in diesem Forum. Einfach mal die Suche verwenden.

    Grüssli



  • Dravere schrieb:

    wodurch man eigentlich immer im Kopf haben sollte, von welchem Typ die Variable war.

    Oder eine gute IDE haben sollte, die beim Mouseover den Typen anzeigt 😉
    Allerdings sollte es in einem gegebenen Kontext eh nicht mehr als ca. 10-20 Variablen geben, so dass man sich nicht so viele Namen merken muss 😉



  • Diotor schrieb:

    Das ganze wird dann wieder in die Textdatei geschrieben
    - Sollte ich die Variablen als Klassenvariablen deklarieren oder den Funktionen übergeben?
    - findeBegegnung() und suchen() als Methoden von Spielplan? formatDatum dafür nicht

    Allgemein gesprochen:
    a) Gehören die Variablen logisch zur Klasse, oder zur Methode?
    b) Erfordern die Methoden interne Daten der Klasse oder nicht? Gehören sie für dich logisch zur Klasse?

    Diotor schrieb:

    - das Array dynamisch? wie?

    z.B. std::vector verwenden.

    Zudem:
    - Deklaration und Definition ist zu trennen.
    - Niemals "using namespace" im Header verwenden.
    - Übergabe von nicht integralen Datentypen würde ich niemals als Kopie, sondern als konstante Referenz umsetzen ("string formatDatum(string)" => "string formatDatum(string const &)")
    - Einrückungen bitte einheitlich machen (unabhängig davon welchen Stil du verwendest).
    Zum Rest (Unsinn der UN etc.) wurde schon etwas gesagt...



  • asc schrieb:

    Diotor schrieb:

    Das ganze wird dann wieder in die Textdatei geschrieben
    - Sollte ich die Variablen als Klassenvariablen deklarieren oder den Funktionen übergeben?
    - findeBegegnung() und suchen() als Methoden von Spielplan? formatDatum dafür nicht

    Allgemein gesprochen:
    a) Gehören die Variablen logisch zur Klasse, oder zur Methode?
    b) Erfordern die Methoden interne Daten der Klasse oder nicht? Gehören sie für dich logisch zur Klasse?

    Diotor schrieb:

    - das Array dynamisch? wie?

    z.B. std::vector verwenden.

    Zudem:
    - Deklaration und Definition ist zu trennen.
    - Niemals "using namespace" im Header verwenden.
    - Übergabe von nicht integralen Datentypen würde ich niemals als Kopie, sondern als konstante Referenz umsetzen ("string formatDatum(string)" => "string formatDatum(string const &)")
    - Einrückungen bitte einheitlich machen (unabhängig davon welchen Stil du verwendest).
    Zum Rest (Unsinn der UN etc.) wurde schon etwas gesagt...

    a) finde sie gehören nicht zur Klasse. Also übergeben?
    b) erfordern interne Daten, findeBegegnung() +suchen() gehört logisch zur Klasse, formatDatum() nicht.



  • Diotor schrieb:

    a) finde sie gehören nicht zur Klasse. Also übergeben?
    b) erfordern interne Daten, findeBegegnung() +suchen() gehört logisch zur Klasse, formatDatum() nicht.

    Du hast du dir die Antwort eigentlich selbst gegeben...



  • Jo, kann ich nix gegen sagen 😃

    Dennoch möchte ich kurz erwähnen wann ich un benutze:
    und zwar genau dann wenn ich zwei verschiedene typen des gleichen kontextes habe und z.B. von a nach b caste...

    achso zur IDE hilfen: Man sollte seinen Code so schreiben das er selbst mit einem nativen text editor lesbar wäre,.. oder?

    Naja,.. aber danke für die info 😉
    ---------------------------------------
    Edit: achso ist ja schon zwo seiten der thread,.. ups,...



  • zeusosc schrieb:

    Dennoch möchte ich kurz erwähnen wann ich un benutze:
    und zwar genau dann wenn ich zwei verschiedene typen des gleichen kontextes habe und z.B. von a nach b caste...

    Inwiefern soll das ein Grund für die Verwendung von UN sein?



  • zeusosc schrieb:

    achso zur IDE hilfen: Man sollte seinen Code so schreiben das er selbst mit einem nativen text editor lesbar wäre,.. oder?

    Also mein Texteditor kann die Syntax hervorheben. Aber selbst wenn dies nicht der Fall ist, halte ich die UN nicht für lesbarkeitsfördernd (und das sogar noch durch deinen Code bestätigt), wichtiger ist das die Bedeutung der Variablen, weniger ihr Typ direkt ablesbar ist. Beispielsweise ist "sArray" unabhängig von UN oder nicht, eine sehr schlechte Namenswahl.

    Zudem liest man Code häufiger, als das man ihn schreibt, und da ist eher wichtig ob der Code sinnvoll zu lesen ist, und man die Bedeutung direkt erfassen kann.



  • ? Warum nicht ?
    Z.b. ich habe eine zeit die einmal als typ int (z.b. in ms) und einmal in form einer strucktur (z.b. schon aufgeschlüsseltg nach std, min, sec etc) gespeichert ist und wenn ich das ding quasi zweimal habe ist es "für mich" übersichtlicher das ding einmal z.b. iStartTime und einmal spStartTime local benenne,...

    Oder gibt es daran etwas auszusetzen?

    greatz 🙂



  • zeusosc schrieb:

    Oder gibt es daran etwas auszusetzen?

    Mach wie du meinst, es gibt genügend Gründe gegen die UN, und diese kann man mit etwas Recherche im Forum und ebenso bei Microsoft finden. In der Mehrheit der aktuellen C++ Projekte (Außer vielleicht bei intensiver WinAPI-Verwendung, oder vielen alten Entwicklern) wirst du nichts von der UN bemerken.



  • Jo gut dann 😃



  • Vor einiger Zeit habe ich einen etwas ausführlicheren Post zur UN geschrieben. Dieser überschneidet sich zwar zu einem grossen Teil mit dem bisher Genannten, aber vielleicht willst du ihn dir ja trotzdem mal anschauen.


Anmelden zum Antworten