Jose Meza
Jose Meza

Reputation: 71

C++ while loop nested if statment

Hi I am having some trouble getting the correct smallest value and the correct largest value . I know it has to do with the while loop. the rest of the program works great. I found the correct average , sum and number of integers

#include<iostream>
#include<fstream>

using namespace std;

int main()
{
    ifstream inputfile;

char choice;


int NumberOfIntegers = 0,
    SumOfIntegers = 0,
    Average = 0 ,
    LargestValue,
    SmallestValue,
    integer;

inputfile.open("random.txt");

if(!inputfile)
{
    cout << "the file could not be open" << endl;
}


inputfile >> integer;

//initialize smallest and largest
SmallestValue = integer;
LargestValue = integer;

while(inputfile)
{
    NumberOfIntegers++;
    SumOfIntegers = SumOfIntegers + integer;

    inputfile >> integer;
    if( integer >> LargestValue || integer << SmallestValue)
    {

        if ( integer >> LargestValue)
            LargestValue = integer;

        else 
            SmallestValue = integer;
    }
}

if(NumberOfIntegers > 0 )
{
    Average = SumOfIntegers / NumberOfIntegers;
}

do
{
    //Display Menu
    cout << "Make a selection from the list" << endl;
    cout << "A.   Get the largest Value" << endl;
    cout << "B.   Get the smallest Value" << endl;
    cout << "C.   Get the sum of the values" << endl;
    cout << "D.   Get the average of the values" << endl;
    cout << "E.   Get the number of values entered" << endl;
    cout << "F.   End this program" << endl << endl;

    cout << "Enter your choice -->  ";
    cin >> choice;

    cout << endl;

    switch (choice)
    {
    case 'a':
    case 'A': cout << "The largest value is " << LargestValue << endl;
        break;

    case 'b':
    case 'B': cout << "The smallest value is " << SmallestValue << endl;
        break;

    case 'c':
    case 'C': cout << "The sum of the values entered is " << SumOfIntegers << endl;
        break;

    case 'd':
    case 'D': cout << "The average of the values entered is " << Average << endl;
        break;

    case 'e':
    case 'E': cout << "The number of values entered is " << NumberOfIntegers << endl;
        break;

    case 'f':
    case 'F': cout << "Program is now ending" << endl;

        return 1;
        break;

    default: 
        cout << choice << " is an invalid value. " << endl;

    }

    cout << endl;

} while( choice != 'f' || choice != 'F');


return 0;

}

Upvotes: 0

Views: 84

Answers (3)

Thomas W
Thomas W

Reputation: 14164

Make your 'largest' & 'smallest' logic blocks separate -- there's no reason or correctness in joining them.

Note that for a list with only one item, that value will be both smallest and largest. Not only is it more complicated, it is incorrect to make these conditions mutually exclusive.

Use better names for your variables. Use "item" or "value" for what you are currently reading, everything is an integer so that is effectively meaningless. I prefer a lowercase first letter for variables.. been working in Java for a while :)

Avoid declaring variables before/ or outside of where they have meaningful value too, where possible.

int value;

inputfile >> value;
if (value > largestValue) {
    largestValue = value;
}
if (value > smallestValue) {
    smallestValue = value;
}

Also, you need to either detect 'first' or initialize smallest/largest to +/- MaxInt to ensure that the first value will be taken up.

Upvotes: 0

dma
dma

Reputation: 1809

You should also make sure you initialize LargestValue to something small and SmallestValue to something large. Otherwise you may miss outliers in your data.

Upvotes: 0

David Schwartz
David Schwartz

Reputation: 182819

integer >> LargestValue

Presumably that should be integer > LargestValue. >> is a shift operation, not a comparison. The same applies to <<.

Upvotes: 2

Related Questions