Filter a list of persons by age and write the results to a relevant file

Let’s say I have a class:

class Data
{
    public string Name { get; set; }
    public string LastName { get; set; }
    public int Age { get; set; }

    public Data(string name, string lastname, int age)
    {
        Name = name;
        LastName = name;
        Age = age;
    }
}

I want to get the age of a person, if it’s <18 then print all the data to Kids.csv, else I want it to be printed to Adults.csv.

I know it’s simple and I managed to do that, but the teacher said that my code didn’t look like objective programming. He told me that I need to use method to return array and create other method for writing the array to file or something.

static void Main(string[] args)
    {
        People[] people = new People[MaxPeople];
        people = ReadData();

        WriteKids(people);
        WriteAdults(people);
    }




static void WriteKids(Data[] people)
{
File.WriteAllText(@"Kids.csv", String.Empty);
using (StreamWriter writer = new StreamWriter(@"Kids.csv", true, Encoding.Default))
{
    for (int i = 0; i < Count; i++)
    {
        if (people[i].Age < 18)
        {
            writer.WriteLine("{0};{1};{2}", people[i].Name,              people[i].LastName, people[i].Age);
        }
    }
}


static void WriteAdults(Data[] people)
{
        File.WriteAllText(@"Adults.csv", String.Empty);
        using (StreamWriter writer = new StreamWriter(@"Adults.csv", true, Encoding.Default))
        {
            for (int i = 0; i < Count; i++)
            {
                if (people[i].Age => 18)
                {
                    writer.WriteLine("{0};{1};{2}", people[i].Name, people[i].LastName, people[i].Age);
                }
            }
        }
 }

• Why is your class called Data? It’s obviously a Person.
• In compound words like lastname each “part” needs to be capitalized, so it should be lastName. You did this correctly with LastName, so be consistent.
• What is People[]? A collection of Peoples? No, it’s a collection of Persons. Also, you didn’t provide us with the code for People, not the code for ReadData();.
• Why define something — People[] people = new People[MaxPeople]; — and then assign it a value on the next line — people = ReadData();? Combine the two lines.
• Why do you use for (int i = 0; i < Count; i++) and not foreach?

Why do File.WriteAllText(@"Kids.csv", String.Empty); and File.WriteAllText(@"Adults.csv", String.Empty);? StreamWriter will create the file if it doesn’t exist

Do you want to clear the file in case it exists? Then use false instead of true for the bool append parameter of your StreamWriter constructor.

Matter of fact, considering you’re using the default encoding and considering you’re overwriting the existing contents if the file exists, why not use a simpler constructor?

WriteKids and WriteAdults do basically the same thing, except for the logic in the if and the filename. So they’re candidates to be replaced by a single method that accepts these as parameter.

Of course, the if logic is hard to represent as a parameter, so that’s an indication you’ve done something wrong. What if the if logic was simply a bool, isAnAdult? That would make it easier.

So the question is: how can you compare something against that bool? The solution: add a (read-only) IsAnAdult property to the Person class, which returns the result of Age > 18.

Of course, I’m again combining things when your teacher told you:

He told me that I need to use method to return array and create other
method for writing the array to file or something.

So you should have:

• one method that accepts Person[] and a bool isAnAdult as parameters and returns Person[] for which isAnAdult is true or false,
• a second method that accepts this filtered Person[] and a filename and creates the file.

I agree with your teacher that you should have the print method do as little else as possible. You should try to have methods that do only one thing because they are easier to read and test. Another thing is to keep your code DRY (Don’t Repeat Yourself). If you have code that is repeated, you should refactor it into a separate method. So you have two reasons to print the results in a separate method. Also, you can replace the loops with LINQ, which is more concise, safer, and easier to read in my opinion. I prefer to explicitly declare whether each method is public or private.

private static void Main(string[] args)
{
    People[] people = new People[MaxPeople];
    people = ReadData();

    var kids = GetKids(people);
    var adults = GetAdults(people);
    WritePeople(kids, @"Kids.csv");
    WritePeople(adults, @"Adults.csv");
}

private static Data[] GetKids(Data[] people)
{
    return people.Where(x => x.Age < 18).ToArray();
}

private static Data[] GetAdults(Data[] people)
{
    return people.Where(x => x.Age >= 18).ToArray();
}

private static void WritePeople(Data[] people, string fileName)
{
    File.WriteAllText(fileName, String.Empty);    
    using (StreamWriter writer = new StreamWriter(fileName, true, Encoding.Default))
    {
        foreach(var person in people)
        {
            writer.WriteLine("{0};{1};{2}", person.Name, person.LastName, person.Age);
        }
    }
}

Why is the class named Data if you call them People? (not familar with C#, this may not be an issue?)

WriteAdults and WriteKids should be a function inside your class

static void writeKids(StreamWriter writer, string fileName){ }

for(int i = 0; i < count; i++){
     people[i].writeKids(...);
}