-1

I finished this code homework assignment tonight. I thought I was done, but I just realized that my "Average" value is coming out wrong with certain values. For example: When my professor entered the values 22, 66, 45.1, and 88 he got an "Average" of 55.27. However, when I enter those values in my program, I get an "Average" of 55.25. I have no idea what I am doing wrong. I was pretty confident in my program until I noticed that flaw. My program is due at midnight, so I am clueless on how to fix it. Any tips will be greatly appreciated!

Code Prompt: "Write a program that dynamically allocates an array large enough to hold a user-defined number of test scores. Once all the scores are entered, the array should be passed to a function that sorts them in ascending order. Another function should be called that calculates the average score. The program should display the sorted list of scores and averages with appropriate headings. Use pointer notation rather than array notation whenever possible."

Professor Notes: The book only states, "Input Validation: Do not accept negative numbers for test scores." We also need to have input validation for the number of scores. If it is negative, including 0, the program halts, we should consider this situation for 'counter' not to be negative while we have a loop to enter numbers. So negative numbers should be rejected for the number of scores and the values of scores.

Here is my code:


#include <iostream>
#include <iomanip>

using namespace std;

void showArray(double* array, int size);
double averageArray(double* array, int size);
void orderArray(double* array, int size);

int main()
{
    double* scores = nullptr;
    int counter;
    double numberOfScores;

    cout << "\nHow many test scores will you enter? ";
    cin >> numberOfScores;

    if (numberOfScores < 0) {

        cout << "The number cannot be negative.\n"
             << "Enter another number: ";
        cin >> numberOfScores;
    }

    if (numberOfScores == 0) {

        cout << "You must enter a number greater than zero.\n"
             << "Enter another number: ";
        cin >> numberOfScores;
    }

    scores = new double[numberOfScores];

    for (counter = 0; counter < numberOfScores; counter++) {

        cout << "Enter test score " << (counter + 1) << ": ";
        cin >> *(scores + counter);
        if (*(scores + counter) < 0) {

            cout << "Negative scores are not allowed. " << endl
                 << "Enter another score for this test : ";
            cin >> *(scores + counter);
        }
    }
    orderArray(scores, counter);

    cout << "\nThe test scores in ascending order, and their average, are: " << endl
         << endl;
    cout << " Score" << endl;
    cout << " -----" << endl
         << endl;

    showArray(scores, counter);

    cout << "\nAverage Score: "
         << " " << averageArray(scores, counter) << endl
         << endl;
    cout << "Press any key to continue...";

    delete[] scores;
    scores = nullptr;

    system("pause>0");
}

void orderArray(double* array, int size)
{
    int counterx;
    int minIndex;
    int minValue;

    for (counterx = 0; counterx < (size - 1); counterx++) {

        minIndex = counterx;
        minValue = *(array + counterx);

        for (int index = counterx + 1; index < size; index++) {

            if (*(array + index) < minValue) {

                minValue = *(array + index);
                minIndex = index;
            }
        }

        *(array + minIndex) = *(array + counterx);
        *(array + counterx) = minValue;
    }
}

double averageArray(double* array, int size)
{

    int x;
    double total{};

    for (x = 0; x < size; x++) {

        total += *(array + x);
    }

    double average = total / size;
    return average;
}

void showArray(double* array, int size)
{

    for (int i = 0; i < size; i++) {

        cout << "     " << *(array + i) << endl;
    }
}
Ken White
  • 123,280
  • 14
  • 225
  • 444
  • 4
    This will sound harsh, but your timeline is of no concern to anyone here but you. Your input validation doesn't validate. If I enter two consecutive bad numbers, you'll just accept the second one. Don't bother with pointer arithmetic, use your array like an array. Again, you blindly accept the second value that's input, which could be negative. While I believe that `double total{};` does zero-initialize, there's nothing wrong with being explicit. – sweenish Jan 17 '22 at 03:28
  • 2
    I would start with a function whose purpose is to ensure a positive double is entered. It's DRY, and easier to debug. You can then rule determine whether your sorting or averaging is the culprit using a known set of numbers that you've hand-calculated. – sweenish Jan 17 '22 at 03:32
  • 2
    The scores are floating point numbers, but your `orderArray(...)` handles them as integers, so it rounds `45.1` to `45`. Did you not notice this in the output? – Beta Jan 17 '22 at 03:38
  • Hi, thank you for responding. Sorry if I phrased my question wrong. I am extremely new to programming and I don't really know what I am doing. So, I need to get rid of my "if statements" that put conditions on the user entered value and use a brand new function instead? – MadisonB1303 Jan 17 '22 at 03:41
  • I thought the scores were being handled as doubles? – MadisonB1303 Jan 17 '22 at 03:43
  • 1
    They're not in your `orderArray()` function. Read the code you wrote - how did you declare `minValue`? – Ken White Jan 17 '22 at 03:44
  • 1
    Unrelated: Stop using *(array + x) and write array[x] if possible. It is less to type and looks more idiomatic. – Öö Tiib Jan 17 '22 at 03:50
  • Yes, I don't like the way it looks either. My professor told us to use pointer notation instead of array notation for this particular assignment this week. – MadisonB1303 Jan 17 '22 at 03:56
  • @MadisonB1303 [What is a debugger?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems). Learn to use the debugger, and you won't have to wait for someone else (StackOverflow volunteers) to debug the code for you. – PaulMcKenzie Jan 17 '22 at 04:30

1 Answers1

1

I try to start my answers with a brief code review:

#include <iostream>
#include <iomanip>

using namespace std;  // Bad practice; avoid

void showArray(double* array, int size);
double averageArray(double* array, int size);
void orderArray(double* array, int size);

int main()
{
    double* scores = nullptr;
    int counter;
    double numberOfScores;

    cout << "\nHow many test scores will you enter? ";
    cin >> numberOfScores;

    // This is not input validation, I can enter two consecutive bad values,
    // and the second one will be accepted.
    if (numberOfScores < 0) {
        // Weird formatting, this blank line
        cout << "The number cannot be negative.\n"
             << "Enter another number: ";
        cin >> numberOfScores;
    }

    // The homework, as presented, doesn't say you have to treat 0 differently.
    if (numberOfScores == 0) {

        cout << "You must enter a number greater than zero.\n"
             << "Enter another number: ";
        cin >> numberOfScores;
    }

    scores = new double[numberOfScores];

    // Declare your loop counter in the loop
    for (counter = 0; counter < numberOfScores; counter++) {

        cout << "Enter test score " << (counter + 1) << ": ";
        cin >> *(scores + counter);
        if (*(scores + counter) < 0) {

            cout << "Negative scores are not allowed. " << endl
                 << "Enter another score for this test : ";
            cin >> *(scores + counter);
        }
    }
    orderArray(scores, counter);  // Why not use numberOfScores?

    cout << "\nThe test scores in ascending order, and their average, are: " << endl
         << endl;
    cout << " Score" << endl;
    cout << " -----" << endl
         << endl;

    showArray(scores, counter);  // Same as above.

    cout << "\nAverage Score: "
         << " " << averageArray(scores, counter) << endl
         << endl;
    cout << "Press any key to continue...";

    delete[] scores;
    scores = nullptr;

    system("pause>0");  // Meh, I suppose if you're on VS
}

void orderArray(double* array, int size)
{
    int counterx;
    int minIndex;
    int minValue;  // Unnecessary, and also the culprit

    // This looks like selection sort
    for (counterx = 0; counterx < (size - 1); counterx++) {

        minIndex = counterx;
        minValue = *(array + counterx);

        for (int index = counterx + 1; index < size; index++) {

            if (*(array + index) < minValue) {

                minValue = *(array + index);
                minIndex = index;
            }
        }

        *(array + minIndex) = *(array + counterx);
        *(array + counterx) = minValue;
    }
}

double averageArray(double* array, int size)
{

    int x;
    double total{};

    for (x = 0; x < size; x++) {

        total += *(array + x);
    }

    double average = total / size;
    return average;
}

void showArray(double* array, int size)
{

    for (int i = 0; i < size; i++) {

        cout << "     " << *(array + i) << endl;
    }
}

When you are sorting your array, you keep track of the minValue as an int and not a double. That's why your average of the sample input is incorrect. 45.1 is truncated to 45 for your calculations. You don't need to keep track of the minValue at all. Knowing where the minimum is, and where it needs to go is sufficient.

But as I pointed out, there are some other serious problems with your code, namely, your [lack of] input validation. Currently, if I enter two consecutive bad numbers, the second one will be accepted no matter what. You need a loop that will not exit until a good value is entered. It appears that you are allowed to assume that it's always a number at least, and not frisbee or any other non-numeric value.

Below is an example of what your program could look like if your professor decides to teach you C++. It requires that you compile to the C++17 standard. I don't know what compiler you're using, but it appears to be Visual Studio Community. I'm not very familiar with that IDE, but I imagine it's easy enough to set in the project settings.

#include <algorithm>
#include <iomanip>
#include <iostream>
#include <numeric>
#include <string>
#include <vector>

// Assumes a number is always entered
double positive_value_prompt(const std::string& prompt) {
  double num;

  std::cout << prompt;
  do {
    std::cin >> num;
    if (num <= 0) {
      std::cerr << "Value must be positive.\n";
    }
  } while (num <= 0);

  return num;
}

int main() {
  // Declare variables when you need them.
  double numberOfScores =
      positive_value_prompt("How many test scores will you enter? ");
  std::vector<double> scores;

  for (int counter = 0; counter < numberOfScores; counter++) {
    scores.push_back(positive_value_prompt("Enter test score: "));
  }

  std::sort(scores.begin(), scores.end());
  for (const auto& i : scores) {
    std::cout << i << ' ';
  }
  std::cout << '\n';
  std::cout << "\nAverage Score: "
            << std::reduce(
                   scores.begin(), scores.end(), 0.0,
                   [size = scores.size()](auto mean, const auto& val) mutable {
                     return mean += val / size;
                   })
            << '\n';
}

And here's an example of selection sort where you don't have to worry about the minimum value. It requires that you compile to C++20. You can see the code running here.

#include <iostream>
#include <random>
#include <vector>

void selection_sort(std::vector<int>& vec) {
  for (int i = 0; i < std::ssize(vec); ++i) {
    int minIdx = i;
    for (int j = i + 1; j < std::ssize(vec); ++j) {
      if (vec[j] < vec[minIdx]) {
        minIdx = j;
      }
    }
    int tmp = vec[i];
    vec[i] = vec[minIdx];
    vec[minIdx] = tmp;
  }
}

void print(const std::vector<int>& v) {
  for (const auto& i : v) {
    std::cout << i << ' ';
  }
  std::cout << '\n';
}

int main() {
  std::mt19937 prng(std::random_device{}());
  std::uniform_int_distribution<int> dist(1, 1000);
  std::vector<int> v;

  for (int i = 0; i < 10; ++i) {
    v.push_back(dist(prng));
  }
  print(v);

  selection_sort(v);
  print(v);
}

I opted not to give your code the 'light touch' treatment because than I would have done your homework for you, and that's just not something I do. However, the logic shown should still be able to guide you toward a working solution.

sweenish
  • 4,793
  • 3
  • 12
  • 23
  • Thanks for your review. I am currently trying to tweak things in Visual Studios. – MadisonB1303 Jan 17 '22 at 03:50
  • I am not understanding how I can fix the problem with minValue. – MadisonB1303 Jan 17 '22 at 03:54
  • Don't use it all. You don't need it. Knowing where the minimum is and where to swap it to is enough. And multiple people now have told you the problem with `minValue`. – sweenish Jan 17 '22 at 04:00
  • That's why I was confused. When I deleted it earlier today, I got an error message stating that minValue was undefined. So, I figured I had to list it. – MadisonB1303 Jan 17 '22 at 04:03
  • 1
    You simply didn't delete all instances of you using the variable. – sweenish Jan 17 '22 at 04:08
  • So, I don't need minValue in my function at all? It's still gonna work the same way? I'm so sorry if I sound stupid, I am just in over my head here. – MadisonB1303 Jan 17 '22 at 04:10
  • Thanks for your help. I really appreciate it. – MadisonB1303 Jan 17 '22 at 04:35
  • You don't sound stupid. My guess is that's it's late and you've been staring at your code for a very long time. If you have the time to spare, get away from your screen for a few minutes, clear your head of the assignment completely, and meditate for a bit. Come back with fresh eyes. I've let the silliest mistakes go under the radar for the same reasons. Clearing your head, and learning to use a debugger are two very important skills to learn. – sweenish Jan 17 '22 at 04:37
  • Yeah, I think I'll do that. – MadisonB1303 Jan 17 '22 at 04:40