I'm creating a piece of code that involves getting the average of 3 variables (skiTime1, skiTime2, and skiTime3), and saving it as the variable skiTime. The 3 variables and the average are saved into a text file. I asked a question on here previously, but I keep getting the text "NaN" in my text file where skiTime (the average) should be.

List<string> skiTime1 = new List<string>();
List<string> skiTime2 = new List<string>();
List<string> skiTime3 = new List<string>();

public string CalculateAverage(List<string> skiTime1, List<string> skiTime2, List<string> skiTime3)
{
    List<string> allValues = new List<string>();
    allValues.AddRange(skiTime1);
    allValues.AddRange(skiTime2);
    allValues.AddRange(skiTime3);
    float totalcount = 0;
    float average = 0;

    foreach (var value in allValues)
    {
        totalcount = totalcount   float.Parse(value);
    }

    average = totalcount / allValues.Count();
    return average.ToString();
}

the variable is going into the text file with the code:

using (StreamWriter sw = new StreamWriter("pupilSkiTimes.txt", true))
{
    string loginDetails = addSkiTimesPupilCB.Text   "~"   addSkiTimesTime1TB.Text   "~"   addSkiTimesTime2TB.Text   "~"   addSkiTimesTime3TB.Text   "~"   skiTime;
    sw.WriteLine(loginDetails);
    sw.Close();
}
MessageBox.Show("Time has been recorded");

CodePudding user response：

This version has some optimizations:

public static string CalculateAverage(List<string> skiTime1, List<string> skiTime2, List<string> skiTime3)
{
    float sum = 0.0f;
    int count = 0;

    var allLists = new[] { skiTime1, skiTime2, skiTime3 };
    foreach (var list in allLists)
    {
        foreach (var text in list)
        {
            float value;
            if (float.TryParse(text, out value))
            {
                sum  = value;
                count  ;
            }
        }
    }

    var average = count == 0 ? 0.0f : sum / count;
    return average.ToString();
}

The methods is static because it's not needed an instance of the class.

I use a list with 3 items (the lists parameters) instead of create a list and copy all the elements of the other lists (that usually will be more items).

Also, I use a variable to count the valid numbers instead of use the Count() method that meybe wrong if any of the items are not a number (you Count every item)

CodePudding user response：

General speaking there are 2 issues,

1. Can you ensure all your string values can be parsed to float, what happens when it is empty or null or none parsable value.
2. Does The sum of totalCount would exceed the float max value limit?

Anyway, I would use Try Parse and validate my numbers.

so in your loop make the following changes:

instead of this

foreach (var value in allValues)
{
    totalcount = totalcount   float.Parse(value);
}

change it to this

foreach (var value in allValues)
{
    bool isParsable = float.TryParse(value, out float number);
    if (isParsable)
    {
        totalcount  = number;
    }
}