Dateien (Frage zu Code)

Guten Abend,
ich bin noch Anfänger und habe gerade folgenden Code geschrieben, er funktioniert aber ich frage mich ob er auch "richtig" ist. Ich meine damit ob ihr etwas daran ändern würdet (Auch Kleinigkeiten) oder ob das ganze ok ist.

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

class A
{
	public:		
		 static char* foo(string DatName);
};

char* A::foo(string DatName)
{
	static char temp[1000];
	ifstream X;
	int i = 0;

	X.open(DatName.c_str(),ios_base::in);
	while (!X.eof())
	{
		temp[i] = X.get();
		++i;
	}
	temp[i] = '\0';

	X.close();
	return temp;
}

int main()
{
	cout << A::foo("c:\\testfile1.abc") << endl;
	cout << A::foo("c:\\testfile2.abc") << endl;
	return 0;
}

Ich freue mich über konstruktive Beiträge.

Mfg Mario

Ja, einen Konstruktor und Destruktor schreiben.
ifstream X als private deklarieren.
Für dataname gilt das gleiche.

z.B:

class A 
{
    private: 
    char temp[1000];
    string m_DatName;
    ifstream m_X;
    public:
    A(string DatName,ifstream X);
    A();
    static char* foo(string DatName); 
};

A::A(string DatName,ifstream X)
:m_DatName(DatName),
 m_X(X);
{
}

A::~A(){
}

Aziz

Hallo,

Ich würd ein Makro definieren, das die Größe des Buffers festlegt.
Bsp:

#define BUFFER_SIZE 1024

Dann würde ich die statische "temp" Variable mit lauter Nullen initialisieren:
static char temp[BUFFER_SIZE] = {0}; //funktioniert ausschließlich mit der Null. {5} würde nur das erste Feld des Arrays initialisieren (is zumindest auf VC++ so)

Wenn du das auf diese Weise initialisierst brauchst du die Variable i nicht mehr, weil ohnehin das Array mit Null-Zeichen gefüllt ist.

Das wärs für's Erste.

Friede,
Aziz

vektoren verwenden wäre nicht schlecht.

MfG Max

Bashar

Warum die Alibi-Klasse? Mach eine freie Funktion daraus.

Zum Code an sich:

ich würde den Konstruktor von ifstream statt open benutzen:

ifstream X(Dateiname.c_str(), ios_base::in);

entsprechend close weglassen und das Schließen der Datei dem Destruktor überlassen
einen besseren Namen als X
Man sollte nicht nur bei EOF abbrechen, sondern bei sämtlichen Fehlerzuständen des Streams. Ausserdem sollte dieser Fehlerzustand erst nach der Eingabe abgefragt werden (weil er auch erst da gesetzt wird):

for (;;) {
  X.get(temp[i]);
  if (!X) break;
  ++i;
}

statischer Speicher ist nicht unbedingt das beste, da zum einen die Funktion nicht mehr reentrant ist (gibt Probleme mit Threads) und zum anderen jeder Aufrufer den Rückgabestring entweder sofort verarbeiten oder aber zunächst kopieren muss, falls er gedenkt, die Funktion nochmal aufzurufen.
die willkürliche und gefährliche (weil nicht abgeprüfte) Begrenzung auf 1000 Zeichen

ok,
ich werde jetzt einmal alle Verbesserungsvorschläge durchgehn und den Code verbessern.
Danach zeig ich ihn euch nochmal.

Danke erstmal für die Tips.

Shade Of Mine

Aziz schrieb:

Ich würd ein Makro definieren, das die Größe des Buffers festlegt.

Fuer Konstanten verwenden wir 'const' und keine haesslichen #defines

Aziz

Fuer Konstanten verwenden wir 'const' und keine haesslichen #defines

Ok, nach kurzer Überlegung erscheint mir das auch vernünftig

Wenn man das nicht mit const macht, dann erstellt der Compiler ständig eine neue Konstante wenn er auf BUFFER_SIZE trifft, oder?

Shade Of Mine

Aziz schrieb:

Wenn man das nicht mit const macht, dann erstellt der Compiler ständig eine neue Konstante wenn er auf BUFFER_SIZE trifft, oder?

Jein.
Der Preprocessor ersetzt BUFFER_SIZE einfach durch die Zahl mit der du BUFFER_SIZE definiert hast. Der Compiler sieht dann ueberall nur die Zahl stehen. Im Prinzip ist es also fuer den Compiler kein grosser Unterschied. Allerdings bringen #defines ja genug Probleme mit sich, so dass wir sie vermeiden wo es nur geht.