The File1.WriteLine
and File2.WriteLine
lines are identical except for File1 vs. File2, I think. If that's actually true, find a way to avoid duplicating the code for both cases.
StreamWriter File1 = new StreamWriter("First.csv", false, Encoding.Default);
StreamWriter File2 = new StreamWriter("Fourth.csv", false, Encoding.Default);
for (int i = 0; i < Count; i++)
{ // still imperative, but not horrible to read.
StreamWriter f = (student[i].Year == 1) ? File1 : File2; // or however C# does references
f.WriteLine(stuff that nobody wants to read/update twice to make sure it's identical);
}
File1.Close();
File2.Close();
Or even (condition ? File1 : File2).WriteLine()
, but that would make one really hard to read line.
An Object Oriented way to write this would at least open the files in the constructor for something, and implicitly close them in the destructor when the object went out of scope. Whether this is more clear or not for opening/closing files is up for debate, but it is the object-oriented way to do things. (And certainly prevents leaking open file descriptors and stuff like that if the function returns early or throws an exception.)
An OO way to do the loop would be for a student object to know how to print itself, and you'd pass it a file descriptor. (And maybe some kind of formatting selector / option to choose which things you wanted it to print).
StreamWriter File1("First.csv", false, Encoding.Default);
StreamWriter File2("Fourth.csv", false, Encoding.Default);
// IDK C#, do you still need new to construct local temporary objects, like Java?
for (...) {
student[i].WriteRecord( (student[i].Year == 1) ? File1:File2, some formating options );
}
// files closed when File1 and File2 go out of scope.
Having a separate piece of code to handle formatting student data into records would be the usual choice, though. Putting that right inside the Student
class combines two separate responsibilities that don't need to go together. Having a function in Student
that takes a StreamWriter
, rather than just returning a string, is probably a bad choice.
There's more to good design than just object-oriented or not.