5
\$\begingroup\$

This program is for a class project and requires me to read data from a text file then display a list of totals. Here are the instructions/requirements:

The fields in each record will be separated by a comma.

Format: Age, Gender, Marital Status, District#

For example: 18, M, S, 2

The city has 22 districts. The census department wants to see a listing of how many residents are in each district, and a count of residents in each of the following age groups (for all the districts combined): under 18, 18 through 30, 31 through 45, 46 through 64, and 65 or older.

The only way I can get this to work was using a bunch of if statements (which is bad I've been told). Is there a way for me to just read through the data and put the correct age/district number in an array, then display the totals for the each Age Group (groups 1-5) and District (1-22)?

So say each time the program reads age 18 and district 2 for example, it will add 1 to Age Group 1 and 1 to District 2 that element and so on? Then at the end just display the results? I just barely learned about array and I don't know how I can do this.

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[] ageData = new int[900];
        int[] districtDataA = new int[900];
        int[] ageGroup = new int[5];
        int[] districtCount = new int[22];
       int i = 0;
       foreach (string line in File.ReadAllLines("test.txt"))
        {
            string[] fields = line.Split(',');

            ageData[i] = int.Parse(fields[0]);          
            districtDataA[i] = int.Parse(fields[3]);


            if (ageData[i] > 0 && ageData[i] <= 18)
            {
              ageGroup[0] = ageGroup[0] + 1;
            }
            if (ageData[i] > 18 && ageData[i] <= 30)
            {
               ageGroup[1] = ageGroup[1] + 1;
            }
            if (ageData[i] > 30 && ageData[i] <= 45)
            {
               ageGroup[2] = ageGroup[2] + 1;
            }
            if (ageData[i] > 45 && ageData[i] <= 64)
            {
                ageGroup[3] = ageGroup[3] + 1;
            }
            if (ageData[i] >= 65)
            {
                ageGroup[4] = ageGroup[4] + 1;
            }

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

        i++;

    }//End For

    Console.WriteLine("This program provides a list of residents in 5 age groups,");
    Console.WriteLine("And a list of residents in each district 1-22");
    Console.WriteLine("(1.)----------------AGE-GROUP-Count----------------");
    Console.WriteLine("Age Group 18 & under = {0}", ageGroup[0]);
    Console.WriteLine("Age Group 18-30 = {0}", ageGroup[1]);
    Console.WriteLine("Age Group 31-45 = {0}", ageGroup[2]);
    Console.WriteLine("Age Group 46-64 = {0}", ageGroup[3]);
    Console.WriteLine("Age Group 65 & over = {0}", ageGroup[4]);
    Console.WriteLine("(2.)--------------COUNT-PER-DISRTRICT--------------");
    Console.WriteLine("District 1 = {0}", districtCount[0]);
    Console.WriteLine("District 2 = {0}", districtCount[1]);
    Console.WriteLine("District 3 = {0}", districtCount[2]);
    Console.WriteLine("District 4 = {0}", districtCount[3]);
    Console.WriteLine("District 5 = {0}", districtCount[4]);
    Console.WriteLine("District 6 = {0}", districtCount[5]);
    Console.WriteLine("District 7 = {0}", districtCount[6]);
    Console.WriteLine("District 8 = {0}", districtCount[7]);
    Console.WriteLine("District 9 = {0}", districtCount[8]);
    Console.WriteLine("District 10 = {0}", districtCount[9]);
    Console.WriteLine("District 11 = {0}", districtCount[10]);
    Console.WriteLine("District 12 = {0}", districtCount[11]);
    Console.WriteLine("District 13 = {0}", districtCount[12]);
    Console.WriteLine("District 14 = {0}", districtCount[13]);
    Console.WriteLine("District 15 = {0}", districtCount[14]);
    Console.WriteLine("District 16 = {0}", districtCount[15]);
    Console.WriteLine("District 17 = {0}", districtCount[16]);
    Console.WriteLine("District 18 = {0}", districtCount[17]);
    Console.WriteLine("District 19 = {0}", districtCount[18]);
    Console.WriteLine("District 20 = {0}", districtCount[19]);
    Console.WriteLine("District 21 = {0}", districtCount[20]);
    Console.WriteLine("District 22 = {0}", districtCount[21]);


}
}
\$\endgroup\$
1
  • 3
    \$\begingroup\$ Please use spaces instead of tabs here. Tabs mess up the markdown. \$\endgroup\$
    – Jamal
    Apr 6, 2014 at 23:03

3 Answers 3

6
\$\begingroup\$

Try to visualize how your items are related. Think how your code runs, step by step, like in slow-mo. You will realize you can sum up long and tedious code into simplest, more abstract, styles.

Let me start from the last portion of your code:

    Console.WriteLine("District 1 = {0}", districtCount[0]);
    Console.WriteLine("District 2 = {0}", districtCount[1]);
    Console.WriteLine("District 3 = {0}", districtCount[2]);
    //etc etc

If you find yourself writing repetitive lines as you have here, there is almost always a better way to code that. Here, a simple loop makes sense:

for (int i = 1; i <= 22; i++)
    {
        Console.WriteLine("District {0} = {1}", i, districtCount[i-1]);
    }

That makes it short and readable, less convoluted. We merely extracted the 1,2,3... and the 0,1,2... into i and i-1 respectively.

Now using the same "trick" you can replace all your ifs (which should have been if else's in the first place) with just a single line:

districtCount[districtDataA[i] - 1]++;

BONUS: When you want to type variable = variable + 1 it's shorter and more readable if you type variable++ instead. Same with variable--. For different amounts, you can do variable += amount (also -=, *=, /= and more operators depending on the language)

\$\endgroup\$
1
\$\begingroup\$

Your if blocks that are detirmining theage bracket can be shortened:

if (ageData[i] > 0 && ageData[i] <= 18)
{
    ageGroup[0] = ageGroup[0] + 1;
}
if (ageData[i] > 18 && ageData[i] <= 30)
{
    ageGroup[1] = ageGroup[1] + 1;
}
if (ageData[i] > 30 && ageData[i] <= 45)
{
    ageGroup[2] = ageGroup[2] + 1;
}
if (ageData[i] > 45 && ageData[i] <= 64)
{
    ageGroup[3] = ageGroup[3] + 1;
}
if (ageData[i] >= 65)
{
    ageGroup[4] = ageGroup[4] + 1;
}

can become and iterative approach by putting the ranges in a data sctructure. Keeping with your style using arrays.

First you declare:

 int[] ageGroup = new int[5];
 int[] ageGroupLimit = new int[] {17, 30, 45, 64, int.MaxValue};

then you loop through the data:

for(int groupID=0; groupID < ageGroupLimit.Length; groupID++)
{
    if(ageData[i] <= ageGroupLimit[groupID])
    {
         ageGroup[groupID]++;
         break;
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Thank you, I am getting an error for the ageGroupLimit.length; saying "'System.Array' does not contain a definition for 'length'..." I'm not sure what the error means. \$\endgroup\$ Apr 7, 2014 at 4:57
  • \$\begingroup\$ @proppaganda it should be .Length If you use an IDE with auto completion/intellisense you should be able to find these things yourself. \$\endgroup\$ Apr 7, 2014 at 4:59
  • \$\begingroup\$ Ah I see, thank you. I did not realize it needed to be capitalized. \$\endgroup\$ Apr 7, 2014 at 5:17
0
\$\begingroup\$

One way to shorten the code determining the age group is to use a Dictionary. the key is the age the value is the agegroup index. One relatively simple way to fill the dictionary is with LINQ:

AgeGroups = (from i in Enumerable.Range(0, 19)
                select new
                {
                    key = i,
                    value = 0
                }).ToDictionary(x => x.key, x => x.value);
AgeGroups = AgeGroups.Concat((from i in Enumerable.Range(19, 12)
                                select new
                                {
                                    Key = i,
                                    Value = 1
                                }).ToDictionary(x => x.Key, x => x.Value)).ToDictionary(x => x.Key, x => x.Value);
AgeGroups = AgeGroups.Concat((from i in Enumerable.Range(31, 15)
                                select new
                                {
                                    Key = i,
                                    Value = 2
                                }).ToDictionary(x => x.Key, x => x.Value)).ToDictionary(x => x.Key, x => x.Value);
AgeGroups = AgeGroups.Concat((from i in Enumerable.Range(46, 19)
                                select new
                                {
                                    Key = i,
                                    Value = 3
                                }).ToDictionary(x => x.Key, x => x.Value)).ToDictionary(x => x.Key, x => x.Value);

This creates a dictionary with 65 elements with the value of each element the appropriate agegroup index.

You build this once.

Shortening the district code is simply matter of subtracting 1 from the district number to get the appropriate index.

If there's a chance the size of the data wil be large, you should consider reading each line separately rather than all of them together.

With all this in mind the code to fill your arrays looks like this:

using(StreamReader lines = new StreamReader("test.txt"))
{
    int i = 0;
    while(!lines.EndOfStream)
    {
        string[] fields = lines.ReadLine().Split(',');

        ageData[i] = int.Parse(fields[0]);          
        districtDataA[i] = int.Parse(fields[3]);

        if(AgeGroups.ContainsKey(ageData[i]))
            ageGroup[AgeGroups[ageData[i]]]++;
        else
            ageGroup[4]++;
        districtCount[districtDataA[i++] - 1]++;
    }
}

Notice how you still need one loop but you're not storing the whole file in memory. Also, the only if statement is to check for the top age group.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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