Problem mit FileStream / StreamReader / StreamWriter

Hallo zusammen,

ich habe ein Problem mit meinem Code. Leider schreibt er nicht die Dateien die ich haben will.

Hier der Code:

Das ist die Funktion die ich aufrufe:

public int MPbind()
{
string file;
string afile;
string path = Globals.path;
string str;
string str1;
	        int i = 0;
	        int j = 0;

	        for (i = 0; i < Globals.number + 1; i++)
	        {
		        Read read = new Read();

		        file = Globals.list[i,0];
		        str = path + "\\" + "test" + "\\" + file + ".PRO";
		        j = 0;
		        while(Globals.list[i,j] != "")
		     	{		        
                    afile = Globals.list[i,j];
			        str1 = path + "\\" + afile + ".PRO";

			        if (j == 0)
			        {
				        read.description(str, str1);
			        }

                    read.read_write(str, str1);
			        j++;
		        }
	        }

	        return 0;
        }

Hier die zwei Funktionen die zur Klasse Read gehören:

public void description(string str, string str1)
        {
            string title1;
            string title2;
            string title3;
            string title4;
            string title5;

            FileStream fs = new FileStream(str, FileMode.Open, FileAccess.Read, FileShare.Read);
            StreamReader sr = new StreamReader(fs);
            title1 = sr.ReadLine();
            title2 = sr.ReadLine();
            title3 = sr.ReadLine();
            title4 = sr.ReadLine();
            title5 = sr.ReadLine();

            sr.Close();

            FileStream fs1 = new FileStream(str1, FileMode.OpenOrCreate, FileAccess.Write, FileShare.Write);
            StreamWriter sw = new StreamWriter(fs1);
            sw.WriteLine(title1);
            sw.WriteLine(title2);
            sw.WriteLine(title3);
            sw.WriteLine(title4);
            sw.WriteLine(title5);

            sw.Close();
        }

public void read_write(string str, string str1)
        {
            string title1;
            string title2;
            string title3;
            string title4;
            string title5;

            FileStream fs = new FileStream(str, FileMode.Open, FileAccess.Read, FileShare.Read);
            StreamReader sr = new StreamReader(fs);

            title1 = sr.ReadLine();
            title2 = sr.ReadLine();
            title3 = sr.ReadLine();
            title4 = sr.ReadLine();
            title5 = sr.ReadLine();

            FileStream fs1 = new FileStream(str1, FileMode.Append, FileAccess.Write, FileShare.Write);
            StreamWriter sw = new StreamWriter(fs1);

            while (!sr.EndOfStream)
            {
                sw.WriteLine(sr.ReadLine());
            }

            fs.Close();
            fs1.Close();
            sr.Close();
            sw.Close();
        }

description soll die 5 Kopfzeilen einlesen, eine Datei erstellen und die Kopfzeilen da rein schreiben. read_write soll weitere Dateien einlesen und diese an die vorher erstellte Datei hängen (exkl. Kopfzeilen).

Leider funktioniert das bei mir nicht. Er erstellt auch keine Dateien.

Ich hoffe ihr könnt mir helfen.

Viele Grüße,
doemi

Wow. Das ist ein Lehrbuchbeispiel für unverständlichen, unwartbaren und fehleranfälligen Code.

"Leider funktioniert das bei mir nicht"

Exception?

Welchen Wert haben str und str1 zur Laufzeit?
Setze einen Breakpoint und schau, ob es gültige Pfade sind. (Path.Combine statt string-Konkatenation!)

Wenn Du mehr Hilfe willst, räum den Code auf oder poste ein kompilierbares Minimalbeispiel. Würde mich nicht wundern, wenn Du den Fehler dann selbst findest. So habe ich jedenfalls keine Lust, mich tiefer reinzudenken.

Nichts für ungut.

DarkShadow44

Also wenn er keine Dateien erstellt wird gibts 2 Gründe:
a) der Pfad falsch und die findest sie einfach nicht dort wo sie sein soll oder
b) die Zeile wird nie aufgerufen

Wie wärs wenn du das mal Schritt für Schritt im Debugger duchgehst ?
Da nur du den kompletten Code hast und damit die zugehörigen globalen Daten kannst eigentlich nur du das machen..

µ schrieb:

Wow. Das ist ein Lehrbuchbeispiel für unverständlichen, unwartbaren und fehleranfälligen Code.

"Leider funktioniert das bei mir nicht"

Exception?

Welchen Wert haben str und str1 zur Laufzeit?
Setze einen Breakpoint und schau, ob es gültige Pfade sind. (Path.Combine statt string-Konkatenation!)

Wenn Du mehr Hilfe willst, räum den Code auf oder poste ein kompilierbares Minimalbeispiel. Würde mich nicht wundern, wenn Du den Fehler dann selbst findest. So habe ich jedenfalls keine Lust, mich tiefer reinzudenken.

Nichts für ungut.

Sorry, bin leider Anfänger was C# betrifft. Hast du evtl. Tipps wie ich den Code "aufräumen" kann?

Beim Debuggen hab ich festgestellt, dass ich die Werte von str und str1 vertauscht habe. Damit konnte es nicht funktionieren.

Die erste Datei wird auch jetzt erstellt, aber es werden nicht alle Zeilen reingeschrieben.

Hat sich erledigt, funktioniert jetzt alles. Danke trotzdem.

Aber für Tipps wie ich den Code aufräumen kann wäre ich weiterhin dankbar.

DarkShadow44

Wie du den Code aufräumen kannst:
-Codeteile die man kürzer Schreiben kann kürzer schreiben
-Unnötige Codeteile weglassen
-Codeteile die öfter in ähnlicher Form auftauchen in Funktionen auslagern
-Variablen wie (titel 1-5) in Array packen falls es geht
-Globale Daten vermeiden, Daten kapseln
-Variablen sinnvoll bennenen
-Code leicht lesbar/verständlich machen

usw.

Danke.

Die Globalen Variablen lassen sich leider nicht vermeiden. ist es sinnvoll auf diese Variablen mit Hilfe von gettern und settern zuzugreifen?

DarkShadow44

Nicht zwangsläufig, ich würde sagen das kommt auf den Fall an. Wenn es jetzt eine Variable ist die nur an einer Stelle geändert wird ist das z.B. weniger sinnvoll als wenn sie an vielen verschiedenen Stellen geändert wird.
Was ich sagen wollte ist dass du - wenn möglich - globale Variablen vermeiden solltest und damit vorsichtig umgehst. Es geht halt nicht immer ohne und ist prinzipiell auch nichts Schlimmes.

P.S.: Was dein Problem eventuell von Anfang an vermieden hätte wäre wenn du gleich verständlicherer Bezeichner statt str/str1 verwendest

DarkShadow44 schrieb:

Nicht zwangsläufig, ich würde sagen das kommt auf den Fall an. Wenn es jetzt eine Variable ist die nur an einer Stelle geändert wird ist das z.B. weniger sinnvoll als wenn sie an vielen verschiedenen Stellen geändert wird.
Was ich sagen wollte ist dass du - wenn möglich - globale Variablen vermeiden solltest und damit vorsichtig umgehst. Es geht halt nicht immer ohne und ist prinzipiell auch nichts Schlimmes.

P.S.: Was dein Problem eventuell von Anfang an vermieden hätte wäre wenn du gleich verständlicherer Bezeichner statt str/str1 verwendest

Ja die Bezeichner habe ich jetzt geändert. Die globalen Variablen ändere ich nur jeweils an einer Stelle im Programm und lasse sie danach unverändert bzw. hole mir nur die Werte.

Danke schön.

doemi schrieb:

Hast du evtl. Tipps wie ich den Code "aufräumen" kann?

Variablen-, Methoden- und Typnamen sollten unbedingt ein Stück "Bedeutung" an den menschlichen Leser vermitteln und v.a. nicht widersprüchlich bzw. inkonsistent (Klasse Read hat Methode read_write) sein (wurde schon genannt).
Variablen dort deklarieren, wo sie benötigt werden und nicht alle am Anfang der Methode. So lokal wie möglich. Du deklarierst sogar Laufvariablen einer for-Schleife vor der Schleife. Hast Du evtl. vor 20 Jahren C gelernt? Es ist wirklich schwierig die erste Initialisierung einer Variablen zu finden, wenn sie nicht direkt bei der Deklaration stattfindet. Fürs Verständis ist sowas aber wichtig.
Händisches Zusammenbauen eines Pfades ist eine potentielle Fehlerquelle. Sehr schnell hat man ein \ zu viel und der Pfad ist ungültig. Es gibt eine Path.Combine-Methode für solche Zwecke. Die ist robust und fängt Fehler ab. Wenn man mehr als zwei strings zu einem Pfad zusammenbauen will, kann man sich einfach eine eigene (Extension-)Methode basteln, die auf Path.Combine basiert und beliebig viele Pfadstücke zusammensetzt.
Die while-Schleife sollte imho eine for-Schleife sein. Bin mir auch nicht sicher, ob Du wirklich beim ersten Globals.list[i,j] == "" (Tipp: string.IsNullOrEmpty-Methode) abbrechen willst. Kommen danach (bezogen auf j) vielleicht noch weitere Einträge?
if(j==0) : Das könntest Du vor der innersten Schleife erledigen. Muss nicht, könnte aber. Hängt davon ab, wie verständlich der Code nach einer Bereinigung ist.
Globale Variablen: Warum lassen sie sich in Deinem Anwendungsfall nicht vermeiden? Sie lassen sich nahezu immer vermeiden und es gibt nur wenige Ausnahmen, in denen sie sinnvoll sind. C# hat eine sehr angenehme Referenzsemantik für Klassen-Objekte. So kann man Referenzen auf Objekte bis tief in alle Anwendungsschichten weiterreichen. Mache Gebrauch davon und Globale Objekte erledigen sich i.d.R. von selbst. Du hast davon einige Vorteile. Unter Anderem kann das Objekt, das Du an irgendwelche Konstruktoren oder Methoden übergibst, jederzeit ausgetauscht werden. Bei Globalen Objekten geht das nicht so einfach.
Die Dispose-Methode sollte aufgerufen werden, falls ein Typ sie anbietet. Z.B. FileStream. Besser: Verwende using-Blöcke dafür. using-Blöcke entsprechen einem try-finally-Konstrukt und garantieren, dass Dispose aufgerufen und somit wichtige Systemressourcen freigegeben werden. Insbesondere im Falle einer Exception.
Verwende Schleifen statt mehrfach den gleichen Code zu schreiben (titleX = sr.ReadLine() und sw.WriteLine(titleX))
Noch ein Punkt für später: Mache Dich irgendwann mit LINQ vertraut. Damit lässt sich oft sehr eleganter Code schreiben, der nicht auf Schleifen-Index-Gefrickel aufbaut.

Lies Dir in einem Monat nochmal Deinen ursprünglichen Code durch. Solange Code im Kurzzeitgedächtnis ist, versteht man ihn, auch wenn er katastrophal ist. Das ist auch häufig Ursache, warum man gerne mal üblen Code schreibt und nicht realisiert, dass er für andere wenig verständlich ist. Ich hatte jedenfalls erhebliche Probleme Deinen Code zu verstehen. Aber wie gesagt: Absolut nichts für ungut

Chiller

Hast du schon mal ReSharper und StyleCop verwendet?
StyleCop ist gratis, soweit ich weis, gibt es auch eine TestVersion von ReSharper.

Mit den beiden Tools machst du dann schon ziemlich hübschen Code und so Sachen wie LINQ werden automatisch angeboten.

Grüsse
Chiller

µ schrieb:

doemi schrieb:

Hast du evtl. Tipps wie ich den Code "aufräumen" kann?

Variablen-, Methoden- und Typnamen sollten unbedingt ein Stück "Bedeutung" an den menschlichen Leser vermitteln und v.a. nicht widersprüchlich bzw. inkonsistent (Klasse Read hat Methode read_write) sein (wurde schon genannt).

Variablen dort deklarieren, wo sie benötigt werden und nicht alle am Anfang der Methode. So lokal wie möglich. Du deklarierst sogar Laufvariablen einer for-Schleife vor der Schleife. Hast Du evtl. vor 20 Jahren C gelernt? Es ist wirklich schwierig die erste Initialisierung einer Variablen zu finden, wenn sie nicht direkt bei der Deklaration stattfindet. Fürs Verständis ist sowas aber wichtig.

Händisches Zusammenbauen eines Pfades ist eine potentielle Fehlerquelle. Sehr schnell hat man ein \ zu viel und der Pfad ist ungültig. Es gibt eine Path.Combine-Methode für solche Zwecke. Die ist robust und fängt Fehler ab. Wenn man mehr als zwei strings zu einem Pfad zusammenbauen will, kann man sich einfach eine eigene (Extension-)Methode basteln, die auf Path.Combine basiert und beliebig viele Pfadstücke zusammensetzt.

Die while-Schleife sollte imho eine for-Schleife sein. Bin mir auch nicht sicher, ob Du wirklich beim ersten Globals.list[i,j] == "" (Tipp: string.IsNullOrEmpty-Methode) abbrechen willst. Kommen danach (bezogen auf j) vielleicht noch weitere Einträge?

if(j==0) : Das könntest Du vor der innersten Schleife erledigen. Muss nicht, könnte aber. Hängt davon ab, wie verständlich der Code nach einer Bereinigung ist.

Globale Variablen: Warum lassen sie sich in Deinem Anwendungsfall nicht vermeiden? Sie lassen sich nahezu immer vermeiden und es gibt nur wenige Ausnahmen, in denen sie sinnvoll sind. C# hat eine sehr angenehme Referenzsemantik für Klassen-Objekte. So kann man Referenzen auf Objekte bis tief in alle Anwendungsschichten weiterreichen. Mache Gebrauch davon und Globale Objekte erledigen sich i.d.R. von selbst. Du hast davon einige Vorteile. Unter Anderem kann das Objekt, das Du an irgendwelche Konstruktoren oder Methoden übergibst, jederzeit ausgetauscht werden. Bei Globalen Objekten geht das nicht so einfach.

Die Dispose-Methode sollte aufgerufen werden, falls ein Typ sie anbietet. Z.B. FileStream. Besser: Verwende using-Blöcke dafür. using-Blöcke entsprechen einem try-finally-Konstrukt und garantieren, dass Dispose aufgerufen und somit wichtige Systemressourcen freigegeben werden. Insbesondere im Falle einer Exception.

Verwende Schleifen statt mehrfach den gleichen Code zu schreiben (titleX = sr.ReadLine() und sw.WriteLine(titleX))

Noch ein Punkt für später: Mache Dich irgendwann mit LINQ vertraut. Damit lässt sich oft sehr eleganter Code schreiben, der nicht auf Schleifen-Index-Gefrickel aufbaut.

Lies Dir in einem Monat nochmal Deinen ursprünglichen Code durch. Solange Code im Kurzzeitgedächtnis ist, versteht man ihn, auch wenn er katastrophal ist. Das ist auch häufig Ursache, warum man gerne mal üblen Code schreibt und nicht realisiert, dass er für andere wenig verständlich ist. Ich hatte jedenfalls erhebliche Probleme Deinen Code zu verstehen. Aber wie gesagt: Absolut nichts für ungut

hab die using Blöcke mal eingebaut:

private void write(string source, string destination)
        {
            using (FileStream fs = new FileStream(source, FileMode.Open, FileAccess.Read, FileShare.Read))
            {
                using (FileStream fs1 = new FileStream(destination, FileMode.Append, FileAccess.Write, FileShare.Write))
                {
                    using (StreamReader sr = new StreamReader(fs, Encoding.Default))
                    {
                        using (StreamWriter sw = new StreamWriter(fs1, Encoding.Default))
                        {
                            string[] title = new string[5];
                            for (int i = 0; i < title.Length; i++)
                            {
                                title[i] = sr.ReadLine();
                            }

                            string line = sr.ReadToEnd();
                            sw.Write(line);

                            sr.Close();
                            sw.Close();
                            fs.Close();
                            fs1.Close();
                        }
                    }
                }
            }
        }

macht man das so?

Die globalen Variablen belege ich während der WindowsForms Anwendung mit Werten. Ich wusste nicht wie ich auf diese Werte anders zugreifen sollte. Hast du evtl. einen Link zum Thema Referenzen, falls das in meiner Situation in Frage kommt?

µ schrieb:

doemi schrieb:

Hast du evtl. Tipps wie ich den Code "aufräumen" kann?

Variablen-, Methoden- und Typnamen sollten unbedingt ein Stück "Bedeutung" an den menschlichen Leser vermitteln und v.a. nicht widersprüchlich bzw. inkonsistent (Klasse Read hat Methode read_write) sein (wurde schon genannt).

Variablen dort deklarieren, wo sie benötigt werden und nicht alle am Anfang der Methode. So lokal wie möglich. Du deklarierst sogar Laufvariablen einer for-Schleife vor der Schleife. Hast Du evtl. vor 20 Jahren C gelernt? Es ist wirklich schwierig die erste Initialisierung einer Variablen zu finden, wenn sie nicht direkt bei der Deklaration stattfindet. Fürs Verständis ist sowas aber wichtig.

Händisches Zusammenbauen eines Pfades ist eine potentielle Fehlerquelle. Sehr schnell hat man ein \ zu viel und der Pfad ist ungültig. Es gibt eine Path.Combine-Methode für solche Zwecke. Die ist robust und fängt Fehler ab. Wenn man mehr als zwei strings zu einem Pfad zusammenbauen will, kann man sich einfach eine eigene (Extension-)Methode basteln, die auf Path.Combine basiert und beliebig viele Pfadstücke zusammensetzt.

Die while-Schleife sollte imho eine for-Schleife sein. Bin mir auch nicht sicher, ob Du wirklich beim ersten Globals.list[i,j] == "" (Tipp: string.IsNullOrEmpty-Methode) abbrechen willst. Kommen danach (bezogen auf j) vielleicht noch weitere Einträge?

if(j==0) : Das könntest Du vor der innersten Schleife erledigen. Muss nicht, könnte aber. Hängt davon ab, wie verständlich der Code nach einer Bereinigung ist.

Globale Variablen: Warum lassen sie sich in Deinem Anwendungsfall nicht vermeiden? Sie lassen sich nahezu immer vermeiden und es gibt nur wenige Ausnahmen, in denen sie sinnvoll sind. C# hat eine sehr angenehme Referenzsemantik für Klassen-Objekte. So kann man Referenzen auf Objekte bis tief in alle Anwendungsschichten weiterreichen. Mache Gebrauch davon und Globale Objekte erledigen sich i.d.R. von selbst. Du hast davon einige Vorteile. Unter Anderem kann das Objekt, das Du an irgendwelche Konstruktoren oder Methoden übergibst, jederzeit ausgetauscht werden. Bei Globalen Objekten geht das nicht so einfach.

Die Dispose-Methode sollte aufgerufen werden, falls ein Typ sie anbietet. Z.B. FileStream. Besser: Verwende using-Blöcke dafür. using-Blöcke entsprechen einem try-finally-Konstrukt und garantieren, dass Dispose aufgerufen und somit wichtige Systemressourcen freigegeben werden. Insbesondere im Falle einer Exception.

Verwende Schleifen statt mehrfach den gleichen Code zu schreiben (titleX = sr.ReadLine() und sw.WriteLine(titleX))

Noch ein Punkt für später: Mache Dich irgendwann mit LINQ vertraut. Damit lässt sich oft sehr eleganter Code schreiben, der nicht auf Schleifen-Index-Gefrickel aufbaut.

Lies Dir in einem Monat nochmal Deinen ursprünglichen Code durch. Solange Code im Kurzzeitgedächtnis ist, versteht man ihn, auch wenn er katastrophal ist. Das ist auch häufig Ursache, warum man gerne mal üblen Code schreibt und nicht realisiert, dass er für andere wenig verständlich ist. Ich hatte jedenfalls erhebliche Probleme Deinen Code zu verstehen. Aber wie gesagt: Absolut nichts für ungut

Habe die using Blöcke eingebaut:

private void write(string source, string destination)
        {
            using (FileStream fs = new FileStream(source, FileMode.Open, FileAccess.Read, FileShare.Read))
            {
                using (FileStream fs1 = new FileStream(destination, FileMode.Append, FileAccess.Write, FileShare.Write))
                {
                    using (StreamReader sr = new StreamReader(fs, Encoding.Default))
                    {
                        using (StreamWriter sw = new StreamWriter(fs1, Encoding.Default))
                        {
                            string[] title = new string[5];
                            for (int i = 0; i < title.Length; i++)
                            {
                                title[i] = sr.ReadLine();
                            }

                            string line = sr.ReadToEnd();
                            sw.Write(line);

                            sr.Close();
                            sw.Close();
                            fs.Close();
                            fs1.Close();
                        }
                    }
                }
            }
        }

macht man das so?

using (FileStream fs = new FileStream(source, FileMode.Open, FileAccess.Read, FileShare.Read), 
			      fs1 = new FileStream(destination, FileMode.Append, FileAccess.Write, FileShare.Write))
using (StreamReader sr = new StreamReader(fs, Encoding.Default))
using (StreamWriter sw = new StreamWriter(fs1, Encoding.Default))
{
	for (int i = 0; i < 5; i++)
		sr.ReadLine();

	sw.Write(sr.ReadToEnd());
}

Oder die Filestreams direkt im Konstructor der StreamReader/Writer erstellen. fs und fs1 brauchst Du ja sonst nicht mehr.
Edit: ^ Schräger Satz. Schau dir die anderen Konstruktoren von Reader/Writer an. Die Filestreams brauchst Du wahrscheinlich nicht.

Danke.

Zu den globalen Variablen habe ich noch eine Frage:

Ich schreibe während der WindowsForms Anwendung Daten in die glob. Variablen rein. Ich weiß nicht wie ich sonst auf eine Variable zugreifen kann, die ich z.B. in Form1 erzeugt habe und in Form2 wieder brauche.

Du hast die Referenzen erwähnt. Würde das in meinem Fall Sinn machen?

EXDW

Der FileStream braucht kein using wenn der StreamReader sowie StreamWriter ein using hat.
Wenn ein Stream[Reader|Writer] geschlossen wird, schließt er den darunter liegenden Stream automatisch mit.

Unabhängig davon, wie µ schon sagte, du brauchst kein FileStream Objekt
http://msdn.microsoft.com/de-de/library/system.io.streamreader.aspx

using (var reader = new StreamReader(source, Encoding.Default))
using (var writer = new StreamWriter(destination, Encoding.Default))
{
    for (int i = 0; i < 5; i++)
        sr.ReadLine();

    sw.Write(sr.ReadToEnd());
}

Hallo,

habe es jetzt geändert.

Wann sollte man prinzipiell

var

benutzen und wann nicht?

EXDW

Geschmackssache.
Ich verwende es immer wenn der Typ redundant ist (weil er rechts direkt da steht).
in den meisten fällen aber wenn mir der genaue Typ völlig egal ist.

ReSharper schlägt es auch oft vor (halte mich aber nicht immer dran, je nach gesunden Menschenverstand)

Z.B.:

var items = from project in Projects
            from document in project.Documents
            from language in document.Languages
            where language.Lcid > 1000
            select new { Project = project.Name, Document = document.Name, Language = language.Name };

foreach (var item in items)
    OnFound(string.Format("Project: {0}, Document: {1}, Language: {2}", item.Project, item.Document, item.Language);

da interessiert mich der genaue Typ überhaupt nicht.

oder auch:

Dictionary<Document, List<Language>> dictionary = new Dictionary<Document, List<Language>>();
foreach(KeyValuePair<Document, List<Language>> pair in openWith)
{
}

vs.

var dictionary = new Dictionary<Document, List<Language>>();
foreach(var pair in openWith)
{
}

Aber dieser Thread sollte keine Grundsatzdiskusion werden, zu var oder nicht var haben wir bestimmt schon ein Thread da.