0

I have been working on this assignment for a couple days now. I have written all the code myself. I'm not looking to cheat or have someone do my work for me, but for the life of me, I can't get this to work correctly.

The problem I'm having is that when I try to average numbers in an array, instead of dividing by just the number of entries, it's dividing by the total allowed entries into the array.

For example. I enter 2 values into an array that can hold 100 values, instead of dividing by 2, it divides by 100.

How do I get it to divide by just the number of entries? Here is what I have:

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

namespace ConsoleApplication6
{
    class Program
    {
        static void InputData(string[] player, int[] score, ref int numPlayer)
        {
            // input items up to the number of the array size
            while (numPlayer < player.Length)
            {
                Console.Write("Enter player name (Q to quit): ");
                player[numPlayer] = Console.ReadLine();
                if ((player[numPlayer] == "Q") || (player[numPlayer] == "q"))
                {
                    Console.WriteLine();
                    break;
                }
                else
                {
                    Console.Write("Enter score for " + player[numPlayer] + ": ");
                    score[numPlayer] = Convert.ToInt32(Console.ReadLine());
                    numPlayer++;
                }
            }
        }


        static void DisplayPlayerData(string[] player, int[] score, int numPlayer)
        {
            Console.WriteLine("Name           Score");
            for (int i = 0; i < numPlayer; i++)
                Console.WriteLine("{0, -16} {1, 8}", player[i], score[i]);
        }


        static double CalculateAverageScore(int[] score, ref int numPlayer)
        {
            double avgScore;
            double total = 0;

            for (int i = 0; i < numPlayer; i++)
            {
                total += score[i];
            }
            avgScore = total / score.Length;
            Console.WriteLine("Average Score:" + avgScore);
            return avgScore;
        }

        static void DisplayBelowAverage(string[] player, int[] score, int numPlayer)
        {
            double avgScore = CalculateAverageScore(score, ref numPlayer);
            Console.WriteLine("Players who scored below average");
            Console.WriteLine("Name           Score");
            for (int i = 0; i < numPlayer; i++)
            {
                if (score[i] < avgScore)
                {
                    Console.WriteLine("{0, -16} {1}", player[i], score[i]);
                }
            }
        }
        static void Main(string[] args)
        {
            //Variables
            string[] player = new string[100];
            int[] score = new int[100];
            int numPlayer = 0;

            InputData(player, score, ref numPlayer);
            DisplayPlayerData(player, score, numPlayer);
            CalculateAverageScore(score, ref numPlayer);
            DisplayBelowAverage(player, score, numPlayer);


            Console.ReadLine();
        }
    }
}
1
  • 2
    I'd recommend using a List here instead of an array. (Edit) In C# (not many other languages though) you can treat a List just like an array. Commented Jun 8, 2015 at 3:41

2 Answers 2

7
  1. You have a numPlayer variables that stands for a number of entered players.
    Just use it.

Replace

        avgScore = total / score.Length;

with

        avgScore = total / numPlayer;
  1. Your code has some very strange points.

For example, you call CalculateAverageScore(score, ref numPlayer); in a Main(). However, you are not using a return value. This method is properly called in a DisplayBelowAverage method. In general, it looks wrong - refs, static-sized array with dynamic number of values, non-format console writeline etc.

Just for you information. Read this once. Maybe some code lines will help you. Maybe, you won't find something new, unknown or interesting.
This is how I would solve this problem:

public class Program
{   
    private const string InputTerminationString = "Q";

    public static void Main()
    {
        List<Player> players = new List<Player>(); // p. 1, 4

        while (true)
        {
            Console.Write("Enter player name ({0} to quit): ", InputTerminationString);
            string name = Console.ReadLine();
            if (name == InputTerminationString) break; // p. 2

            Console.Write("Enter score for {0}: ", name); // p. 3
            int score = int.Parse(Console.ReadLine());

            players.Add(new Player { Name = name, Score = score }); 
        }

        Console.WriteLine("Name           Score");
        players.ForEach(x => Console.WriteLine("{0, -16} {1, 8}", x.Name, x.Score)); // p. 5

        double average = players.Average(x => x.Score); // p. 6
        Console.WriteLine("Average score: {0:F2}", average); // p. 3

        Console.WriteLine("Players who scored below average");
        Console.WriteLine("Name           Score");
        players
            .Where(x => x.Score < average) // p. 7
            .ToList()
            .ForEach(x => Console.WriteLine("{0, -16} {1, 8}", x.Name, x.Score)); // p. 5           
    }
}

public class Player
{
    public string Name { get; set; }
    public int Score { get; set; }  
}

Now, step by step:

  1. Use Player class. It is pretty inconvenient to pass two independent arrays of names and scores. Moreover, it is not safe and proper in general. A name and a score of a player are properties if a single player and should be stored together either in struct or class.

  2. Use constants. If you need to change 'Q' termination string to 'Exit', you will be able to do this in a second without looking through the code.

  3. Use formatted Console.WriteLine. It works liks String.Format and you don't need to concatenate strings.
    Some info here: https://msdn.microsoft.com/en-us/library/828t9b9h(v=vs.110).aspx

  4. Use dynamic collection List. Array is good for storing and rapid accessing to range of values of known length. Since a user enters values one by one, you never know if he will enter 0, 1 or 90000 values. List collection will help you with it.
    Some info here: http://www.dotnetperls.com/list

  5. You can use ForEach method which executes the given code for every item in a collection. Using ForEach with lambda-expressions makes code shorter and more readable.
    Some info here: https://msdn.microsoft.com/en-us/library/bwabdf9z(v=vs.110).aspx

  6. Use Average function which calculates the average of a collection. In case of int[] you can use arr.Average(). However, in case of Class object, you need to describe the logic of calculation of average value using lambda-expression.
    Some info here: http://www.dotnetperls.com/average

  7. LINQ Where expression lets you filter your collection.
    Some info here: http://www.dotnetperls.com/where

Sign up to request clarification or add additional context in comments.

Comments

3

Instead of writing CalculateAverageScore the you can make use of LINQ's Average method:

double avgScore = score.Average();
Console.WriteLine("Average Score:" + avgScore);

Forthermore it is better not to mix calculating with I/O, so:

static double CalculateAverageScore(int[] score) {
     return score.Average();
}

Or since you use an array, use the sum:

static double CalculateAverageScore(int[] score, ref int numPlayer) {
     return (double) score.Sum()/numPlayer;
}

and do the printing in the Main:

static void Main(string[] args) {
    //Variables
    string[] player = new string[100];
    int[] score = new int[100];
    int numPlayer = 0;

    InputData(player, score, ref numPlayer);
    DisplayPlayerData(player, score, numPlayer);
    double avgScore = CalculateAverageScore(score, ref numPlayer);
    Console.WriteLine("Average Score:" + avgScore);
    DisplayBelowAverage(player, score, numPlayer);
    Console.ReadLine();
}

online Ideone demo.

You furthermore better modify the array to a List. Here there is also no reason to use the ref keyword in all these methods, nor to make them static.

5 Comments

Definitely the best way, but the professor/teacher might not like it. S/He probably wants it done by hand.
@RogueCSDev: well the OP's code contains all kinds of weird aspects like making everything static, using ref where not supposed to, printing aspects inside methods, etc. I would not be very satisfied with this answer at all. I agree LINQ is not really the way to solve homework. But it is a valid alternative I think. Furthermore some students keep implementing these themselves.
I'm sorry for a stupid question but what do you mean by "printing aspects inside methods"? I'm not familiar with the term "aspect". What is it? Thank you.
@YeldarKurmangaliyev: Console.WriteLine's. A program should make a distinction between methods that calculate something; and methods that do semething. Otherwise you cannot reuse the method. Say you want to calculate the average of the scores of the persons below the first average, for instance...
@CommuSoft, Oh, now I see. F.i., Console.WriteLine("Average Score:" + avgScore) in a method called CalculateAverageValue. Thank you again. I understand that it is absolutely wrong but I never knew it is called this way and never heard the "aspect" term outside of AOP term.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.