HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
agethefilepersonswriterelevantfilterresultsandlist

Problem

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  18)
                {
                    writer.WriteLine("{0};{1};{2}", people[i].Name, people[i].LastName, people[i].Age);
                }
            }
        }
 }

Solution


  • 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



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.

Context

StackExchange Code Review Q#105499, answer score: 6

Revisions (0)

No revisions yet.