Ist dieser Code "schön"?

vikenemesh

Ich schrieb folgendes Programm zur Zählung der Buchstabenhäufigkeiten als Übung.

Ist der Stil in dem ich programmiere okay oder mach ich irgendwelche Kardinalsfehler?

/* ###### 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 end();

int main (int argc, char * argv[]) {
	ifstream file;

	int count[26]={0};
	int inchar;

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

	file.open(argv[1]);
	if (!file) {
		cout<<"Dateifehler!\n";
		return end();
	}

	while (!file.eof()) {
		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<<"\n";

	return end();
}

int end() {
	cout<<"Enter drücken zum Beenden...";
	cin.ignore(numeric_limits<streamsize>::max(),'\n');
	return 0;
}

Ich bedanke mich schonmal für eure Meinung

zeusosc

nimm mal beim newline noch ein carriage return dazu,...

vikenemesh

Okay, mit welcher Begründung?
Funktionell ist das ja kein Unterschied.

theliquidwave

[c]return end();[/c] ist völliger Blödsinn. [c]return 0[/c]
Wahlweise kannst du [c]std::end[/c] auch anstatt von [c]\n[/c] benutzen. Die Ausgabe wird damit sofort geflusht, d.H. sofort ausgegeben.

Edit: Vergiss es

Das mit dem \r liegt daran, dass Windows in Dateien normalerweise \r\n als Zeilenumbruch benutzt. Wenn du aber Binärdateien hast, ist dies nicht unbedingt der Fall. Selbiges, wenn du eine Datei aus Linux-Umgebung herunterlädst oder mit einem Texteditor (Beispielsweise Notepad++) einstellst, dass du Unix-Zeilenänderungen haben möchtest. Also vorsichtig damit.

vikenemesh

Der Spaß mit \r\n ist mir bekannt, aber ich benutze hier ja nur die Konsolenstreams im Zusammenhang mit \n, also bin ich richtig in der Annahme, dass ich es hier bei \n belassen kann?

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

vikenemesh

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?

C++ Forumbot

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.

HighLigerBiMBam

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

vikenemesh

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

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.

wx++

Der C++-Header heißt cctype

SeppJ

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

vikenemesh

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

DocShoe

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.

zeusosc

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

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.

Shade Of Mine

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.

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.

Shade Of Mine

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

Ist dieser Code &quot;schön&quot;?

Ist dieser Code "schön"?