functions

[DJL]

Hey there,

So Im working on a problem that finds the average score and drops the lowest score (we just started getting into functions).

I think I have it mostly worked out however one of the challenges is I need to have the function getScore call upon the bool function isValid to validate the score entered.

basically the getScore function needs to have the user enter a score then run that score through isValid to find out if its true or false. if true then the score goes on to be calculated.

Im having trouble figuring out how to have getScore call on and run isValid.

I thought what I did on line 15 would work, however no such luck.

anyways figured I would see you guys had some ideas.

  #include <iostream>
#include <iomanip>
using namespace std;

bool isValid(double, double );
void getScore(double &score);
void calcAverage(double, double, double, double, double);

int main (){
    double score1, score2, score3, score4, score5;
    double maxValue = 100;

    cout << endl;

    getScore(isValid(score1)); 
    getScore(score2);
    getScore(score3);
    getScore(score4);
    getScore(score5);

    calcAverage(score1, score2, score3, score4, score5);

return 0;
}

bool isValid (double score, double maxValue){
    if (score < maxValue){
        return true;
        if (score > maxValue){
            return false;} }
}


void getScore(double &score, double maxValue){
        cout << "Enter your test score ";
        cin >> score;
        
}

double findLowest(double score1, double score2, double score3, double score4, double score5, double lowest){
        lowest = score1;
            if (score2 < lowest)
                lowest = score2;

                    else if (score3 < lowest)
                            lowest = score3;

                                else if (score4 < lowest)
                                        lowest = score4;

                                        else if (score5 < lowest)
                                                    lowest = score5;

                    cout << endl;
                    cout << "Your lowest score was  " << lowest << endl;

                    return lowest;}


void calcAverage (double score1, double score2, double score3, double score4, double score5){

            double findLowest(double, double, double, double, double, double);
                double lowest;
                    double average;

                findLowest(score1, score2, score3, score4, score5, lowest);

                average = (((double)score1 + score2 + score3 + score4 + score5) - lowest) / 4.0;

                cout << setw(4);
                cout << fixed << showpoint << setprecision(2);
                cout << endl;
                cout << "Your averaged four highest scores were  " << average << endl;
                cout << endl;
                cout << endl; }

[MikeyBoy]

At line 15, you're trying to pass the value returned by isValid() into getScore. However, isValid() returns a bool, and getScore doesn't take a bool as an argument.

In every one of your calls to getScore, you're passing a single argument in. However, you've defined getScore to take 2 arguments, so you need to give it those 2 arguments.

Also, your declaration for getScore is different from your definition for getScore.

[DJL]

Oh gotcha, Yes I realize now I left the maxValue in getScore from a previous attempt of trying to solve this problem.

How would I go about creating getScore in a way the accepts bool?

Im not sure I fully understand returning value/ functions yet.

[MikeyBoy]

How would I go about creating getScore in a way the accepts bool?

void getScore(bool my_bool);

but I don't see why you'd want to. What would you imagine that function doing?

[DJL]

The idea would be that bool acts as the validator to find if the score entered is > 0 and < 100.

sorta a weird way to go about it. Originally I created the getScore function with an if statement that verified if the score entered was correct or not.

My teacher wanted us to create a bool function though to validate the score.

Ill mess around with your suggestions though and see if I can this program working.

[MikeyBoy]

The idea would be that bool acts as the validator to find if the score entered is > 0 and < 100.

bool is just a value that can be true or false. How can you know what value to pass into getScore(), if you haven't even gotten the score yet (because that's what the function does).

Surely you can only validate the score after you've read it from the user's input?

My teacher wanted us to create a bool function though to validate the score.

That seems sensible. But until you have the score you want to validate, you can't call the function to validate it, right?

[DJL]

yea thats where I have been stumped because it doesn't make sense. unless I am completely misunderstanding whats being asked.

[MikeyBoy]

All you've been asked to do is have the getScore function:

1) Read the score from the user
2) Call the isValid function to validate it

[DJL]

So i would to build the isValid function into getScore? something like this

void getScore(double &score, bool isValid){
        cout << "Enter your test score ";
        cin >> score;
        cout << isValid;   
}

[MikeyBoy]

There is no function called isValid in that code snippet. WHat you've done is defined a boolean argument. That is not a function, it's a variable. It's a single value.

Look, just have getScore() call the isValid() function. You clearly know how to call functions, as you do it several times in the code you've posted. So do it here too.

[DJL]

oh sweet! I think I understand now. This seemed to work

getScore(score1); 
    cout << isValid(score1,maxValue);

[TheToaster]

You know, your isValid function is unnecessarily long. It can be simplified to a single statement:

bool isValid (double score, double maxValue) { return score <= maxValue; }

Also, I would suggest that your findLowest function pass an array or vector of all the values and recursively find the minimum in a loop, or better yet, use a standard library algorithm like std::min_element(). The way you have implemented it is unecessary.

It's usually best to remain clear and less verbose in your intent. Though there are certain instances where being verbose in your code is accepted *cough* C++ standards committee *cough*

std::super_duper_owning_resource_ptr<std::the_standard_is_terrified_of_naming_conflicts_map<int>> ptr{};

Of course at this point I'm just joking around.

[DJL]

that is a better suggestion,

im not super great at making things simple and streamline yet since im trying to understand the basics of what it is im trying to accomplish.

im diving into arrays and vectors next week, so dont really know much about them at this point.

bool isValid (double score, double maxValue){
    if (score < maxValue){
        cout << "Valid" << endl;
        return score; }
        else if (score > maxValue){
            cout << "Enter a value between 0 and 100 " << endl;
            cin >> score; } }

I put in this code for the bool statement which seems messy but I guess functional

[Ganado]

Nitpicky note: Your indentation make it look like the else if statement is an inner part of the if statement. Un-indent lines 5, 6, 7 by one indentation.

[TheToaster]

Another not so nitpicky note: Your function fails to do anything if your score is equal to max score. Both of the branches will fail and isValid will output absolutely nothing. Secondly, your function is supposed to return a bool, but you changed it here to return a double, which is not a bool. This is technically incorrect but will work since it will be implicitly converted to bool.

Furthermore, if the else branch is executed, there is no return statement there. This will most likely be warned by your compiler if you have such settings enabled.

Once again, you don't need to overcomplicate things here:

bool isValid (double score, double maxValue) { return score <= maxValue; }

You should do the inputting outside of this function, perhaps in another function called checkValid, or perhaps within main itself.

[Ganado]

Funny thing is I noticed that problem with the original function posted, but then my mind got side-tracked by focusing on the indentation and I forgot about the real issue xD

[DJL]

oh good suggestion, Thanks. readability is something I want to work towards being better at.

[DJL]

Here is what I corrected the program to. Let me know what you guys think, or if there is anything I could do better. Thanks

#include <iostream>
#include <iomanip>
using namespace std;

bool isValid(double, double );
void getScore(double &score);
void calcAverage(double, double, double, double, double);

int main (){
    double score1, score2, score3, score4, score5;
    double maxValue = 100;

    cout << endl;

    getScore(score1); 
    cout << isValid(score1,maxValue);
        if (score1 > maxValue){
            cout << "Enter a Score between 0 and 100" << endl;
            cin >> score1;
        }
    getScore(score2);
    cout << isValid(score2, maxValue);
        if (score2 > maxValue){
            cout << "Enter a Score between 0 and 100" << endl;
            cin >> score2;
        }
    getScore(score3);
    cout << isValid(score3, maxValue);
        if (score3 > maxValue){
            cout << "Enter a Score between 0 and 100" << endl;
            cin >> score3;
        }
    getScore(score4);
    cout << isValid(score4, maxValue);
        if (score4 > maxValue){
            cout << "Enter a Score between 0 and 100" << endl;
            cin >> score4;
        }
    getScore(score5);
    cout << isValid(score5, maxValue);
        if (score5 > maxValue){
            cout << "Enter a Score between 0 and 100" << endl;
            cin >> score5;
        }

    calcAverage(score1, score2, score3, score4, score5);

return 0;
}

bool isValid (double score, double maxValue){
    return score <= maxValue; 
}
            


void getScore(double &score){
        cout << "Enter your test score ";
        cin >> score;}

double findLowest(double score1, double score2, double score3, double score4, double score5, double lowest){
        lowest = score1;
            if (score2 < lowest)
                lowest = score2;

                    else if (score3 < lowest)
                            lowest = score3;

                                else if (score4 < lowest)
                                        lowest = score4;

                                        else if (score5 < lowest)
                                                    lowest = score5;

                    cout << endl;
                    cout << "Your lowest score was  " << lowest << endl;

                    return lowest;}


void calcAverage (double score1, double score2, double score3, double score4, double score5){

            double findLowest(double, double, double, double, double, double);
                double lowest;
                    double average;

                findLowest(score1, score2, score3, score4, score5, lowest);

                average = (((double)score1 + score2 + score3 + score4 + score5) - lowest) / 4.0;

                cout << setw(4);
                cout << fixed << showpoint << setprecision(2);
                cout << endl;
                cout << "Your averaged four highest scores were  " << average << endl;
                cout << endl;
                cout << endl; }

[MikeyBoy]

Looks solid to me. The only thing I can see that's actually wrong with it, is that you're passing lowest as an argument into findLowest(), but doing nothing with it. You immediately overwrite the value you pass in, and you return the result of the calculation back as a return value, so the argument is useless.

Some thoughts for improvements you could make:

1. There's a lot of repeated stuff there. Your code for getting and validating score 1 is identical to that for getting scores 2, 3, etc. It would be better to write a function to do that, to remove the duplicated code.

2. Whenever you find yourself doing something like:

double score1, score2, score3, score4, score5;

and doing exactly the same thing with each of those variables, it's almost always the case that you're better off using a vector (or an array), and using a loop to repeat the same operation on each element of the vector.

2.5 Also, if you switch to using a vector, you can then generalize your findLowest() function to work with any number of variables (i.e. any size of vector).

3. Ganado's comment about indentation still stands. Proper code layout isn't just some nitpicky aspect of presentation - it can be extremely helpful in making it easier for you to see and understand the flow of control through your code, and to spot errors. Adopting a sensible indentation style early on, and sticking with it, is something you will find helpful as you write more and more complex code.

4. I'd strongly encourage you to always use braces for your if and else blocks, even when the block is only 1 line. Omitting the braces leaves you prone to introducing errors later on, if you add more lines to those blocks and forget to add braces.

5. It's legal to do what you've done with your prototypes, declaring the argument types but not their names. But I'd prefer to have the argument names in there too, because it helps document what those arguments are for. It's not so important when it's all in one file like this, but when you start writing more complicated code, and putting the prototypes into header files, it's helpful.