Thread: Max/min function not working correctly

  1. #1
    Registered User
    Join Date
    Oct 2008
    Posts
    14

    Max/min function not working correctly

    Currently working on coding a class which takes in an arbitrary list of integers and performs various functions on the list.
    • Get max
    • Get min
    • Get average
    • Amount of numbers in list
    • Search for integer in list


    So far, I've gotten the code below, but my max/min is not working. It gives a result of the 2nd integer entered in the list and only gives one result ie. I want it to give both the max and min. Have a long way to go yet but if I could that working it would be great help. Here's my code so far

    Code:
    #include <iostream>
    using namespace std;
    // prototype declarations
    class list
    {
    public:
    	int sumArray(int integerArray[], int sizeOfloatArray);
    	int maxArray(int integerArray[], int sizeOfloatArray);
    	float avgArray(int integerArray[], int sizeOfloatArray);
    	void displayArray(int integerArray[], int sizeOfloatArray);
    	
    };
    
    int main()
    {
    	list l;
    	cout << "This program sums values entered by the user\n";
    	cout << "Terminate the loop by entering a negative number\n" << endl;
    	// store numbers into an array
    	int inputValues[128];
    	int numberOfValues;
    	int integerValue;
    	
    	for(numberOfValues = 0;	numberOfValues < 128; numberOfValues++)
    		{
    		// fetch another number
    		cout << "Enter integer: ";
    		cin >> integerValue;
    		// if it’s negative...
    		if (integerValue < 0)
    			{
    			// ...then exit
    			break;
    			}
    			// ... otherwise store the number
    			// into the storage array
    			inputValues[numberOfValues] = integerValue;
    			}
    	
    	// now output the values and the sum of the values
    	l.displayArray(inputValues, numberOfValues);
    
    	cout << "The sum is " << l.sumArray(inputValues, numberOfValues) << endl;
    	cout << "The average is " << l.avgArray(inputValues, numberOfValues) << endl;
    	cout << "No. of elements: " << numberOfValues << endl;
    	cout << "The maximum is " << l.maxArray(inputValues, numberOfValues) << endl;
    	
    
    	// wait until user is ready before terminating program
    	// to allow the user to see the program results
    
    	return 0;
    	}
    
    	// displayArray - display the members of an
    	// array of length sizeOfloatArray
    	void list::displayArray(int integerArray[], int sizeOfArray)
    	{
    		cout << "The value of the array is:" << endl;
    		for (int i = 0; i < sizeOfArray; i++)
    		{
    		cout << i << ": " << integerArray[i] << endl;	
    		}
    	}
    
    	// sumArray - return the sum of the members of an
    	// integer array
    	int list::sumArray(int integerArray[], int sizeOfArray)
    	{
    		int sum = 0;
    		for (int i = 0; i < sizeOfArray; i++)
    			{
    			sum += integerArray[i];
    			}
    		return sum;
    	}
    
    	int list::maxArray(int integerArray[], int sizeOfArray)
    	{
    		int max = integerArray[0];
    		for (int i=0; i<sizeOfArray; i++){
    			if (integerArray[i] > max) {
    				max=integerArray[i]; 
    			}
    		return max;
    		}
    	}
    
    	
    
    	/*int list::minArray(int integerArray[], int sizeOfArray)
    	{
    		int min = integerArray[0];
    		for (int i=0; i<sizeOfArray; i++){
    			if (integerArray[i] < min) {
    				min = integerArray[i]; 
    			}
    		return min;
    		}
    	}*/
    
    	float list::avgArray(int integerArray[], int sizeOfArray)
    	{
    		float sum = 0;
    		for (int i = 0; i < sizeOfArray; i++)
    			{
    			sum += integerArray[i];
    			}
    			return sum/sizeOfArray;
    	}
    Last edited by En-Motion; 03-18-2009 at 05:01 PM. Reason: Make code legible..

  2. #2
    int x = *((int *) NULL); Cactus_Hugger's Avatar
    Join Date
    Jul 2003
    Location
    Banks of the River Styx
    Posts
    902
    First, indent your code. Something like:
    Code:
    int main() {
        code;
        code;
        if(code) {
            code;
        }
    }
    It will make your life so much easier, because your code will be legible. Edit your post when you're done.

    You need to rethink your maxArray function. First - a function gets 1 return value. You can send back other info through pointers/references, but only return 1 item. Should a maxArray() function return a minimum? Probably not. Move that to a minArray() function. Then rethink your loop - do you want to return if you find a value larger/smaller than your current supposed max/min? ...or continue the loop, and see if there is a larger one somewhere down the way?
    long time; /* know C? */
    Unprecedented performance: Nothing ever ran this slow before.
    Any sufficiently advanced bug is indistinguishable from a feature.
    Real Programmers confuse Halloween and Christmas, because dec 25 == oct 31.
    The best way to accelerate an IBM is at 9.8 m/s/s.
    recursion (re - cur' - zhun) n. 1. (see recursion)

  3. #3
    Registered User
    Join Date
    Oct 2008
    Posts
    14
    Cheers for your reply. I've edited my post and also incuded some other functions which now work. This is my max function now but it still doesn't work:

    Code:
    int list::maxArray(int integerArray[], int sizeOfArray)
    	{
    		int max = integerArray[0];
    		for (int i=0; i<sizeOfArray; i++){
    			if (integerArray[i] > max) {
    				max=integerArray[i]; 
    			}
    		return max;
    		}
    	}
    I really can't see where I'm going wrong. As far as I'm aware I'm setting the first element in my array to be the max and then checking each elemnt and then setting it to be the max if it is bigger. But each time my max comes out as the first element. I understand what your saying about continuing the loop but I can't see the error I'm making. If you could nudge me in the right direction I'd be grateful

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Here is your function with indentation that is not so misleading:
    Code:
    int list::maxArray(int integerArray[], int sizeOfArray)
    {
        int max = integerArray[0];
        for (int i=0; i<sizeOfArray; i++){
            if (integerArray[i] > max) {
                max=integerArray[i]; 
            }
            return max;
        }
    }
    Do you see the problem now?
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Oct 2008
    Posts
    14
    Oh my god, I was tearing my hair out! Cheers for that!
    This works perfect now:

    Code:
    int list::maxArray(int integerArray[], int sizeOfArray)
    	{
    		int max = integerArray[0];
    		for (int i=0; i<sizeOfArray; i++){
    			if (integerArray[i] > max) {
    				max=integerArray[i]; 
    			}
    		}
    		return max;
    	}

  6. #6
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    I'd use std::min_element() & std::max_element(), but I'm assuming this is for a school assignment where you have to do it the long way.
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  7. #7
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Quote Originally Posted by En-Motion View Post
    Oh my god, I was tearing my hair out! Cheers for that!
    This works perfect now:

    Code:
    int list::maxArray(int integerArray[], int sizeOfArray)
    	{
    		int max = integerArray[0];
    		for (int i=0; i<sizeOfArray; i++){
    			if (integerArray[i] > max) {
    				max=integerArray[i]; 
    			}
    		}
    		return max;
    	}
    The only true mistake is one you do not learn from. So you're going to have perfect indentation from now on...?!?
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Problem with Visual C++ Object-Oriented Programming Book.
    By GameGenie in forum C++ Programming
    Replies: 9
    Last Post: 08-29-2005, 11:21 PM
  2. C++ compilation issues
    By Rupan in forum C++ Programming
    Replies: 1
    Last Post: 08-22-2005, 05:45 AM
  3. Please Help - Problem with Compilers
    By toonlover in forum C++ Programming
    Replies: 5
    Last Post: 07-23-2005, 10:03 AM
  4. Replies: 3
    Last Post: 03-04-2005, 02:46 PM
  5. qt help
    By Unregistered in forum Linux Programming
    Replies: 1
    Last Post: 04-20-2002, 09:51 AM