5
\$\begingroup\$

I'm still a beginner, so I'm still learning. I'm trying to take test scores for 3 different students, enter them in an array, then display the name, test scores and the average. Trying to also have reusable code for the average. I'm not even sure I'm going about it the right way. This is what I've got so far. It works, but its probably sloppy. I'm sure there are other ways of doing it, but some constructive criticism would be nice.

static void Main(string[] args)
    {
        const int STUDENTS = 3;
        string[] names = new string[STUDENTS];
        names[0] = "Morgan";
        names[1] = "Bowie";
        names[2] = "Ananya";

        Console.WriteLine("Enter test scores for " + names[0]);
        const int MORGAN_TEST = 3;
        int[] morganScores = new int[MORGAN_TEST];

        morganScores[0] = int.Parse(Console.ReadLine());
        morganScores[1] = int.Parse(Console.ReadLine());
        morganScores[2] = int.Parse(Console.ReadLine());
        int sumMorgan = morganScores.Sum();

        Console.WriteLine("Enter test scores for " + names[1]);
        const int BOWIE_TEST = 5;
        int[] bowieScores = new int[BOWIE_TEST];

        bowieScores[0] = int.Parse(Console.ReadLine());
        bowieScores[1] = int.Parse(Console.ReadLine());
        bowieScores[2] = int.Parse(Console.ReadLine());
        bowieScores[3] = int.Parse(Console.ReadLine());
        bowieScores[4] = int.Parse(Console.ReadLine());
        int sumBowie = bowieScores.Sum();

        Console.WriteLine("Enter test scores for " + names[2]);
        const int ANANYA_TEST = 4;
        int[] ananyaScores = new int[ANANYA_TEST];

        ananyaScores[0] = int.Parse(Console.ReadLine());
        ananyaScores[1] = int.Parse(Console.ReadLine());
        ananyaScores[2] = int.Parse(Console.ReadLine());
        ananyaScores[3] = int.Parse(Console.ReadLine());
        int sumAnanya = ananyaScores.Sum();

        double[] average = { MORGAN_TEST, BOWIE_TEST, ANANYA_TEST };

        Console.WriteLine(names[0] + " " + morganScores[0] + " " + morganScores[1] + " " + morganScores[2] + " Average score: {0:N2}", sumMorgan / average[0]);
        Console.WriteLine(names[1] + " " + bowieScores[0] + " " + bowieScores[1] + " " + bowieScores[2] + " " + bowieScores[3] + " " + bowieScores[4] + " Average score: {0:N2}", sumBowie / average[1]);
        Console.WriteLine(names[2] + " " + ananyaScores[0] + " " + ananyaScores[1] + " " + ananyaScores[2] + " " + ananyaScores[3] + " " + " Average score: {0:N2}", sumAnanya / average[2]);
        Console.Read();
    }
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

You've got students and their scores here. I assume that student without scores is meaningless, as well as scores without students. It would be nice to combine that data into single object:

public class StudentScores {
    
    // Auto-property guarantees that student name defined once in constructor
    // and will not accidentally change.
    public string Name { get; }
        
    // Scores count can be different for different students.
    // Private field guarantees that scores will not be messed up by other classes.
    // Otherwise some student could access scores and change them, 
    // which would lead to errors.
    private readonly List<int> scores = new List<int>();

    public StudentScores(string name) {
        Name = name;
    }

    // We still need a method to add scores to our encapsulated list.
    // We could implement Clear() or RemoveAt() public methods later.
    public void AddScore(int score) {
        scores.Add(score);
    }

    public double Average() {
        if (scores.Count > 0)
            return scores.Average();
        else
            return 0;

        // Or simple: return scores.Count > 0 ? scores.Average() : 0;
    }

    // Nicer way to print data: student name and all scores separated by empty space.
    public override string ToString() {
        return $"{Name} {string.Join(" ", scores)}";
    }
}

Without comments it's only 20 lines long:

public class StudentScores
{
    public string Student { get; }

    private readonly List<int> scores = new List<int>();

    public StudentScores(string student)
        => Student = student;

    public void AddScore(int score)
        => scores.Add(score);

    public double Average()
        => scores.Count > 0 ? scores.Average() : 0;

    public override string ToString()
        => $"{Student} {string.Join(" ", scores)}";
}

Now your syntax in Main is much nicer:

static void Main(string[] args) {

    string[] names = {"Morgan", "Bowie", "Ananya"};

    var morgan = new StudentScores(names[0]);

    // Determine how much scores has student.
    Console.WriteLine("Enter number of test scores for " + names[0]);
    int count = int.Parse(Console.ReadLine());
    
    for (int i = 0; i < count; i++) {
       morgan.AddScore(int.Parse(Console.ReadLine()));
    }

    var bowie = new StudentScores(names[1]);
    // ...
    
    var ananya = new StudentScores(names[2]);
    // ...

    Console.WriteLine(morgan.ToString() + " Average score: {0:N2}", morgan.Average());
    Console.WriteLine(bowie.ToString() + " Average score: {0:N2}", bowie.Average());
    Console.WriteLine(ananya.ToString() + " Average score: {0:N2}", ananya.Average());
}

You could improve your Main() further creating single loop for all students:

static void Main(string[] args) {

    string[] names = {"Morgan", "Bowie", "Ananya"};
    List<StudentScores> scores = new List<StudentScores>();

    foreach(var name in names) 
    {
        var student = new StudentScores(name);
        Console.WriteLine($"Enter number of test scores for {name}");
        int count = int.Parse(Console.ReadLine());
        for (int i = 0; i < count; i++)
           student.AddScore(int.Parse(Console.ReadLine()));
        scores.Add(student);
    }

    foreach(var score in scores)
        Console.WriteLine($"{score.ToString()}, Average score: {score.Average():N2}");

You could remove Average() method and move calculations directly to ToString(), but I think it's nice to have methods for such things.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ For demonstration purpose, it would be nice if you use block body instead of expression body. Also, it would better if you add a way to expose the scores to be reusable. Also, suggesting of IEnumerable<int> over List<int> would be a plus. \$\endgroup\$
    – iSR5
    Feb 28, 2020 at 11:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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