3. Ist dieser Code "schön"?

Ist dieser Code "schön"?

  • ?

    Das carriage return macht deinen Code unportabel, lass das.

    ifstream file;n
...
file.open(argv[1]);

    würde ich folgendermassen abändern:

    ifstream file(argv[1]);

    Dann ersparst du dir die Initialisierung von file wenn argc<2 . Allgemein sollte man die Variablendeklaration so lange wie möglich hinauszögern.

    if (argc<2) {
    cout<<"Nicht genug Parameter!\n";
    return end();
}

    Warum** return ** end(); ? Bei falschen Argumenten hätte ich (a) so etwas wie "Aufruf: buchstabenzaehler [DATEI]" ausgegeben und (b) einen von 0 verschiedenen Rückgabewert genommen.
    end() stört bei normaler Konsolennutzung und ich hätte das in der endgültigen Version rausgenommen (für Debugzwecke ist es in Ordnung).

    int count[26]={0}; // lass das ={0} weg

    while (!file.eof()) // alternativ:
while (file)

    Bei end() fehlt ein <<std::flush

    0
  • V

    Mein Code sieht jetzt so aus.

    /* ###### Buchstaben zählen
	## Zählt die Häufigkeiten der Buchstaben eines Textdokuments
	## Autor: Viken Emesh
	## Datum: 06.01.2011
*/
#define WIN32_LEAN_AND_MEAN

#include <iostream>
#include <fstream>
#include <string>
#include <limits>
#include <ctype.h>

using namespace std;

int main (int argc, char * argv[]) {
	if (argc<2) {
		cout<<"Nicht genug Parameter!\n";
		return -1;
	}

	ifstream file(argv[1]);
	if (!file) {
		cout<<"Dateifehler!\n";
		return -1;
	}

	int count[26]={0};
	int inchar;
	while (file) {
		inchar= toupper(file.get());
		if (inchar>='A' && inchar<='Z') {
			count[inchar-'A']++;
		}
	}
	file.close();

	cout<<"Häufigkeiten der Buchstaben:\n";
	for(int i=0;i<26;i++) {
		cout<<(char)(i+'A')<<": "<<count[i]<<"\n";
	}

	cout.flush();
	return 0;
}

    Das "int count[26]={0}" benutze ich um alle Elemte auf 0 zu initialisieren, das muss ich doch machen damit "count[i]++" hinhaut, oder?

    0
  • C

    Dieser Thread wurde von Moderator/in SideWinder aus dem Forum DOS und Win32-Konsole in das Forum C++ (auch C++0x) verschoben.

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

    Dieses Posting wurde automatisch erzeugt.

    0
  • H

    Das define kannst du entfernen, sowie include string und limits. Ich sehe nicht das diese in deinem Code verwendet werden.

    0
  • V

    Oh danke, hatte ich noch übersehen.
    Ansonsten stimmt jetzt alles?

    0
  • ?

    Das mit dem Variablendeklaration so lange wie möglich hinauszögern lässt sich auch auf inchar anwenden.
    und der flush war nur vonnöten, weil du die Zeile noch nicht fertig geschrieben hast. Jetzt brauchts den nicht mehr.

    Bei dem Hinweis zu int count[26]={0} habe ich einen Fehler gemacht, ich hätte bei mir nämlich std::vector<int> count(26) verwendet und da ist automatisch alles auf 0. Ich habe irgendwie gedacht, das sei selbst bei POD-Arrays der Fall...

    Errors könntest du auf cerr ausgeben.

    Viel mehr fällt mir eigentlich nicht auf.

    0
  • W

    Der C++-Header heißt cctype

    0
  • Mod

    Das file.close(); brauchst du nicht. Der Destruktor des fstream-Objekts schließt die Datei. Nutze dies.

    0
  • V

    Danke an alle.
    In Zukunft werde ich die Tipps dann umsetzten.

    0

  • Vielleicht wäre std::map<char,unsigned int> hübscher als ein rohes C-style Array. Damit musst du keinen Index mehr verbiegen und hast eine direkete Zuordnung von Buchstabe zu dessen Anzahl.

    0
  • Z

    encourriage schrieb:

    Das carriage return macht deinen Code unportabel, lass das.

    ?? Das ist nicht ganz richtig, geschichtlich gesehen ist der cursor bei der
    ausgabe von \n nicht nach vorne gesetzt worden,..
    Der carrier musste nach vorne mittels \r gesetzt werden.
    Halt die pure Funktionalität der Steuerungszeichen (ASCII) ist Aufgrund von
    Abwärtskompatibilität bei Konsolenanwendungen (und EXAKT bei externer perephiere) heute noch zu Gewährleisten.

    Daher ist die genannte "unportabilität" eigentlich nur durch verwendung eines unvollständigen satzes an steuerzeichen vorhanden...

    grüße

    0
  • ?

    du könntest noch return -1; für "Nicht genug Parameter" und return -2; für "Dateifehler" machen.

    Beim Parameterfehler sollte er sagen wie es richtig geht.

    0
  • S

    zeusosc schrieb:

    ?? Das ist nicht ganz richtig, geschichtlich gesehen ist der cursor bei der
    ausgabe von \n nicht nach vorne gesetzt worden,..
    Der carrier musste nach vorne mittels \r gesetzt werden.
    Halt die pure Funktionalität der Steuerungszeichen (ASCII) ist Aufgrund von
    Abwärtskompatibilität bei Konsolenanwendungen (und EXAKT bei externer perephiere) heute noch zu Gewährleisten.

    Daher ist die genannte "unportabilität" eigentlich nur durch verwendung eines unvollständigen satzes an steuerzeichen vorhanden...

    Blödsinn. Sorry.

    Prinzipiell stimmt es ja was du schreibst - aber das ist alles irrelevant. \n ist die korrekte Schreibweise hier. Denn dafür sorgt das System. \r\n kann zuviel machen (muss aber nicht). zB auf OS 9 wäre \r\n falsch.

    Deshalb: \n ist die korrekte und portable Form.

    0
  • ?

    Shade Of Mine schrieb:

    Deshalb: \n ist die korrekte und portable Form.

    Ist nicht eher std::endl die korrekte und portable Form? Macht halt leider noch ein flush dazu, sollte hier aber nicht weh tun. Ich kenne es so, dass die Zeilentrenner wie folgt sind: Windows \r\n, Linux \n, MacOS \r.

    0
  • S

    fdgdfg schrieb:

    Shade Of Mine schrieb:

    Deshalb: \n ist die korrekte und portable Form.

    Ist nicht eher std::endl die korrekte und portable Form? Macht halt leider noch ein flush dazu, sollte hier aber nicht weh tun. Ich kenne es so, dass die Zeilentrenner wie folgt sind: Windows \r\n, Linux \n, MacOS \r.

    Nein \n in einem Text-Kontext ist schon OK. Wenn du mit einer binaer Datei (ios::binary) arbeitest, ist \r\n und \n unter windows unterschiedlich - im Text-Kontext nicht.

    Und unter OS X hast du auch \n als Line Ending. \r war unter OS 9

    0
  • ?

    Frage: wenn \n unter Windows durch \r\n ersetzt wird, wäre dann ein \r\n selbst dort falsch? Es müsste doch zu \r\r\n geändert werden.

    0
  • ?

    Shadow of Mine hat es doch schon erläutert. Der einzige Grund für das Text-Flag bei Dateioperationen, ist der unterschiedliche Zeilenumbruch. Ein Zusätzliches Carriage Return \r tut aber nicht weh. Es veranlasst ja nur, dass der Cursor an den Zeilenanfang geht, dort ist er aber schon.

    0
  • N

    Paul Müller schrieb:

    Ein Zusätzliches Carriage Return \r tut aber nicht weh.

    Kommt wohl sehr drauf an, wie der Editor das interpretiert. Warum überhaupt ein '\r' zuviel, wenn '\n' reicht?

    0
  • ?
     

    #include <iostream>
#include <fstream>
#include <cctype>
#include <map>

typedef std::map<char, int> Occurrences;

int main(int argc, char *argv[])
{
	if (argc < 2)
	{
		std::cerr << "Zu wenig Parameter uebergeben" << std::endl;

		return -1;
	}

	std::ifstream file("a.txt");

	if (!file.good())
	{
		std::cerr << "Datei konnte nicht geoffnet werden" << std::endl;

		return -2;
	}

	Occurrences occurrences;

	for (int i = 0; i < 26; ++i)
	{
		occurrences['A' + i] = 0;
	}

	int counted = 0;

	while (!file.eof())
	{
		char ch = file.get();

		if (isalpha(ch))
		{
			++occurrences[toupper(ch)];

			++counted;
		}
	}

	std::cout.precision(3);

	for (Occurrences::iterator it = occurrences.begin(); it != occurrences.end(); ++it)
	{
		std::cout << it->first << " = " << it->second << " (" << (static_cast<float>(it->second) / counted) * 100.f << "%)" << std::endl;
	}
}
    0
  • A

    trzrtztr schrieb:

    ...

    Schön 👍
    Persönlich mag ich statt

    if (!file.good())

    so was:

    if (0 == file.good())

    Und statt

    for (int i = 0; i < 26; ++i)

    das:

    size_t i = 0u;
...
for (i = 0u; i < 26u; ++i)

    ints mag ich überhaupt nicht 🙂

    0
Anmelden zum Antworten