Summarizing a CSV file containing census data

Question

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;

namespace Project2
{
    class Program
    {
        static void Main(string[] args)
        {
            int[] ageForPeople = new int[10000];
            int[] DistrictForCensus = new int[10000];
            int[] agesForCensusGroups = new int[5];
            int[] maxDistrictAmount = new int[22];
            int placeValueHolder = 0;
            foreach (string line in File.ReadAllLines("census.txt"))
            {
                string[] fields = line.Split(',');

                ageForPeople[placeValueHolder] = int.Parse(fields[0]);
                DistrictForCensus[placeValueHolder] = int.Parse(fields[3]);


                if (ageForPeople[placeValueHolder] > 0 && ageForPeople[placeValueHolder] <= 18)
                {
                    agesForCensusGroups[0]++;
                }
                if (ageForPeople[placeValueHolder] > 18 && ageForPeople[placeValueHolder] <= 30)
                {
                    agesForCensusGroups[1]++;
                }
                if (ageForPeople[placeValueHolder] > 30 && ageForPeople[placeValueHolder] <= 45)
                {
                    agesForCensusGroups[2]++;
                }
                if (ageForPeople[placeValueHolder] > 45 && ageForPeople[placeValueHolder] <= 64)
                {
                    agesForCensusGroups[3]++;
                }
                if (ageForPeople[placeValueHolder] >= 65)
                {
                    agesForCensusGroups[4]++;
                }

                //District Count info
                if (DistrictForCensus[placeValueHolder] == 1)
                {
                    maxDistrictAmount[0]++;
                }
                if (DistrictForCensus[placeValueHolder] == 2)
                {
                    maxDistrictAmount[1]++;
                }
                if (DistrictForCensus[placeValueHolder] == 3)
                {
                    maxDistrictAmount[2]++;
                }
                if (DistrictForCensus[placeValueHolder] == 4)
                {
                    maxDistrictAmount[3]++;
                }
                if (DistrictForCensus[placeValueHolder] == 5)
                {
                    maxDistrictAmount[4]++;
                }
                if (DistrictForCensus[placeValueHolder] == 6)
                {
                    maxDistrictAmount[5]++;
                }
                if (DistrictForCensus[placeValueHolder] == 7)
                {
                    maxDistrictAmount[6]++;
                }
                if (DistrictForCensus[placeValueHolder] == 8)
                {
                    maxDistrictAmount[7]++;
                }
                if (DistrictForCensus[placeValueHolder] == 9)
                {
                    maxDistrictAmount[8]++;
                }
                if (DistrictForCensus[placeValueHolder] == 10)
                {
                    maxDistrictAmount[9]++;
                }
                if (DistrictForCensus[placeValueHolder] == 11)
                {
                    maxDistrictAmount[10]++;
                }
                if (DistrictForCensus[placeValueHolder] == 12)
                {
                    maxDistrictAmount[11]++;
                }
                if (DistrictForCensus[placeValueHolder] == 13)
                {
                    maxDistrictAmount[12]++;
                }
                if (DistrictForCensus[placeValueHolder] == 14)
                {
                    maxDistrictAmount[13]++;
                }
                if (DistrictForCensus[placeValueHolder] == 15)
                {
                    maxDistrictAmount[14]++;
                }
                if (DistrictForCensus[placeValueHolder] == 16)
                {
                    maxDistrictAmount[15]++;
                }
                if (DistrictForCensus[placeValueHolder] == 17)
                {
                    maxDistrictAmount[16]++;
                }
                if (DistrictForCensus[placeValueHolder] == 18)
                {
                    maxDistrictAmount[17]++;
                }
                if (DistrictForCensus[placeValueHolder] == 19)
                {
                    maxDistrictAmount[18]++;
                }
                if (DistrictForCensus[placeValueHolder] == 20)
                {
                    maxDistrictAmount[19]++;
                }
                if (DistrictForCensus[placeValueHolder] == 21)
                {
                    maxDistrictAmount[20]++;
                }
                if (DistrictForCensus[placeValueHolder] == 22)
                {
                    maxDistrictAmount[21]++;
                }

                placeValueHolder++;

            }//End For

            Console.WriteLine("This provides a list of residents in 5 different age groups,");
            Console.WriteLine("Also it provides a list of residents in each of the district 1-22");
            Console.WriteLine("-------Ages of people, in all districts-------");
            Console.WriteLine("Age Group 18 & under = {0}", agesForCensusGroups[0]);
            Console.WriteLine("Age Group 18-30 = {0}", agesForCensusGroups[1]);
            Console.WriteLine("Age Group 31-45 = {0}", agesForCensusGroups[2]);
            Console.WriteLine("Age Group 46-64 = {0}", agesForCensusGroups[3]);
            Console.WriteLine("Age Group 65 & over = {0}", agesForCensusGroups[4]);
            Console.WriteLine("-------Amount of people in each district-------");
            for (int z = 1; z <= 22; z++)
            {
                Console.WriteLine("District {0} = {1}", z, maxDistrictAmount[z - 1]);
            }


        }
    }

}

This is what I have so far, I do not know how to further condense the if statements. For example: the districtForCensus displays sequentially all districts. Is there a way to make it go one after another, without 22 if statements?

Answer 1

1. The first if group can be slightly simplified using the else if:

   if (ageForPeople[placeValueHolder] <= 0)
   {
       // Trigger error.
   }
   else if (ageForPeople[placeValueHolder] <= 18)
   {
       agesForCensusGroups[0]++;
   }
   else if (ageForPeople[placeValueHolder] <= 30)
   {
       agesForCensusGroups[1]++;
   }
   else if (ageForPeople[placeValueHolder] <= 45)
   {
       agesForCensusGroups[2]++;
   }
   else if (ageForPeople[placeValueHolder] <= 64)
   {
       agesForCensusGroups[3]++;
   }
   else
   {
       agesForCensusGroups[4]++;
   }
   

2. The second if group can be replaced with the following single line:

   maxDistrictAmount[DistrictForCensus[placeValueHolder] - 1]++;
   

   To make sure that DistrictForCensus[placeValueHolder] is within the allowed bounds:

   // District Count info
   int district = DistrictForCensus[placeValueHolder];
   if (district >= 1 && district <= 22)
   {
       maxDistrictAmount[district - 1]++;
   }
   

3. Side note. Do not use capitalized names for local variables. Rename DistrictForCensus to districtForCensus.

Answer 2

This review is in addition to Dmitry's.

---

You do these lookups multiple times: ageForPeople[placeValueHolder], DistrictForCensus[placeValueHolder], yet outside the foreach they aren't used. Why then define them outside the foreach, and why is this an array? Why not simply assign the result of int.Parse(fields[0]) and int.Parse(fields[3]) to a local variable?

---

agesForCensusGroups[0] etc doesn't mean anything. IMHO this should be a Dictionary<T,T> with a meaningful enum as the key. That way you can simply loop through the enum's values and use its Description attrribute to generate the output.

---

This is still a fairly short program with limited functionality, but you could already consider moving both the agesForCensusGroups logic and the maxDistrictAmount logic each in a method of their own, perhaps even to a class of their own. Keep your Main lean and clean, use it to stitch together the various independent parts. Ditto for the output part: move that to a method of its own, and provide parameters with all the necessary data.

Answer 3

When you have cascading if-condition logic like you have in your first if-block, consider using a lookup table instead. Lookup tables pre-process the data in a way that makes the use of the data much faster.

How would this work? First, define where your boundaries are:

private static readonly int[] ageBounds = {18, 30, 45, 64};

The above array will be used to say: Anything less than or equal to the value at index X will have have bucket X. We use this table to build the actual lookup table:

private static int[] BuildAgeMap()
{
    int[] ret = new int[ageBounds[ageBounds.Length - 1]];
    int limit = 0;
    for (int i = 0; i < ret.Length; i++) {
        if (i > ageBounds[limit]) {
            limit++;
        }
        ret[i] = limit;
    }

    return ret;
}

public static readonly int[] AGES = BuildAgeMap();

What does this accomplish? We now have an array called AGES, with the contents:

0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,...
1,1,1,1,1,1,1,1,1,1,1,1,...
2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,...
3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3

Now, for someone who is age 33, you can get their group by going to AGES[33].

Put this in a simple wrapper method (that takes care of large ages), and you have:

public static int GetAgeGroup(int age) {
    return age >= AGES.Length ? ageBounds.Length : AGES[age];
}

Now, in your CSV parsing, you just need to do:

agesForCensusGroups[GetAgeGroup( .... )]++;

Note that there is no if-statement at all!

You can see how this all works in the following ideone example;