-4
\$\begingroup\$

Does this method follow the rules of objective programming? If not, how can I change it?

static void SaveStudentData(Student[] student)
{
    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++)
    {
        if (student[i].Year == 1)
        {
            File1.WriteLine("{0};{1};{2};{3};{4};{5};{6}", student[i].FName, student[i].LName, student[i].BDate.ToString("yyyy/MM/dd"), student[i].StudentID, student[i].Year, student[i].PhoneNumber, student[i].IsFreshman);
        }
        else if (student[i].Year == 4)
        {
            File2.WriteLine("{0};{1};{2};{3};{4};{5};{6}", student[i].FName, student[i].LName, student[i].BDate.ToString("yyyy/MM/dd"), student[i].StudentID, student[i].Year, student[i].PhoneNumber, student[i].IsFreshman);
        }
    }
    File1.Close();
    File2.Close();
}
\$\endgroup\$

closed as unclear what you're asking by Ethan Bierlein, SirPython, Hosch250, Legato, CodeYogi Sep 24 '15 at 7:28

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 4
    \$\begingroup\$ Sure doesn't. It's 100% imperative. \$\endgroup\$ – Jesse C. Slicer Sep 23 '15 at 19:59
  • \$\begingroup\$ can you tell me how can I change it to be object-oriented \$\endgroup\$ – Reed Sep 23 '15 at 20:07
  • \$\begingroup\$ yep, I made an answer. \$\endgroup\$ – Jesse C. Slicer Sep 23 '15 at 23:32
  • 1
    \$\begingroup\$ Also note, calling it CSV when they're separated by semicolons is wrong on one side or the other... \$\endgroup\$ – Jesse C. Slicer Sep 23 '15 at 23:33
3
\$\begingroup\$

Here's a possible solution - note there are always many ways to solve a problem. The main points I'm trying to demonstrate here are:

  • Your concerns are kept separate. There are data elements, data formatting, I/O and business logic.

  • Classes can be mostly independent. For instance, you can create a different formatting class that implements IStudentFormatter and inject it into the I/O class.

That being said, here is the implementation:

public interface IStudentFormatter
{
    string AsCsv(Student student);
}

public sealed class StudentToCsv : IStudentFormatter
{
    public string AsCsv(Student student)
    {
        if (student == null)
        {
            throw new ArgumentNullException("student");
        }

        return string.Format(
            "{0};{1};{2};{3};{4};{5};{6}",
            student.FName,
            student.LName,
            student.BDate.ToString("yyyy/MM/dd"),
            student.StudentID,
            student.Year,
            student.PhoneNumber,
            student.IsFreshman);
    }
}

public static class StudentBusinessLogic
{
    private const int FirstYear = 1;

    private const int FourthYear = 4;

    public static IEnumerable<Student> FirstYears(this IEnumerable<Student> students)
    {
        if (students == null)
        {
            throw new ArgumentNullException("students");
        }

        return students.Where(student => (student != null) && (student.Year == FirstYear));
    }

    public static IEnumerable<Student> FourthYears(this IEnumerable<Student> students)
    {
        if (students == null)
        {
            throw new ArgumentNullException("students");
        }

        return students.Where(student => (student != null) && (student.Year == FourthYear));
    }

}

public sealed class StudentWriter : StreamWriter
{
    private readonly IStudentFormatter _StudentToCsv;

    public StudentWriter(string path, bool append, Encoding encoding, IStudentFormatter studentToCsv) : base(path, append, encoding)
    {
        if (studentToCsv == null)
        {
            throw new ArgumentNullException("studentToCsv");
        }

        this._StudentToCsv = studentToCsv;
    }

    public void WriteStudent(Student student)
    {
        if (student == null)
        {
            throw new ArgumentNullException("student");
        }

        this.WriteLine(this._StudentToCsv.AsCsv(student));
    }

    public void WriteStudents(IEnumerable<Student> students)
    {
        if (students == null)
        {
            throw new ArgumentNullException("students");
        }

        foreach (var student in students)
        {
            this.WriteStudent(student);
        }
    }
}

You'd call these as such:

static void SaveStudentData(IEnumerable<Student> students)
{
    var studentToCsv = new StudentToCsv();

    using (var file1 = new StudentWriter("First.csv", false, Encoding.Default, studentToCsv))
    {
        file1.WriteStudents(students.FirstYears());
    }

    using (var file2 = new StudentWriter("Fourth.csv", false, Encoding.Default, studentToCsv))
    {
        file2.WriteStudents(students.FourthYears());
    }
}
\$\endgroup\$
3
\$\begingroup\$

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.

\$\endgroup\$
2
\$\begingroup\$

Ideally in OOP the object handles everything about the object.

  • Object-oriented approach: Add a function to your student class which will takes a streamWriter as a parameter, then prints to that streamWriter

  • Not a object-oriented approach, but a little better: Make a function in the student class which returns a string which that can be printed to the file writer.

\$\endgroup\$
  • 1
    \$\begingroup\$ Your point #1 violates the object-oriented Single Responsibility Principle - the student class itself shouldn't know how to write stuff to files. This should be a totally different class. Same goes with the second, for that matter. \$\endgroup\$ – Jesse C. Slicer Sep 23 '15 at 21:43
  • \$\begingroup\$ @JesseC.Slicer: good point, updated my answer acordingly. \$\endgroup\$ – Peter Cordes Sep 23 '15 at 23:01

Not the answer you're looking for? Browse other questions tagged or ask your own question.