Thread: C++ Problem Solving

  1. #1
    Registered User
    Join Date
    Oct 2007
    Posts
    4

    C++ Problem Solving

    The FindMax() function given below is used to find the maximum value in an array of integers which has a start address in memory defined by the pointer pVals. The number of elements in the array is defined by the parameter NumEls. This function compiles and runs but the produces an incorrect result.

    What is wrong with the function?


    Code:
    int FindMax(int * pVals, int NumEls)
    {
    	int i, Max = *pVals++;
    	
    	for (i=1; i<NumEls; ++i)
    	{
    	                   if(Max < *pVals++)
    			Max == *pVals;
                    }
    	return Max;
    }

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    What do you think is wrong with it?

  3. #3
    Registered User
    Join Date
    Apr 2007
    Posts
    102
    well why don't you try going through the function and seeing whats going on. It's pretty easy to see if you look at it well enough.
    My Favorite Programming Line:
    Code:
    #define true ((rand() % 2) ? true : false)

  4. #4
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,210
    Turn up the warning level on your compiler. You should be warned about this.

  5. #5
    Officially An Architect brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,396
    Quote Originally Posted by scojan View Post
    What is wrong with the function?
    Two things... One of them is, what happens when NumEls is 0? The other will be revealed if you actually listen to the compiler warnings.

  6. #6
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    You could also save time and just use std::max_element()

  7. #7
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    Actually, 3 things are wrong, and I don't think the third will be found by turning up compiler warnings. However, it will be found if you just think through the code step by step.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  8. #8
    Registered User
    Join Date
    Oct 2007
    Posts
    4
    Defining ‘i = 1’ in the line:
    Code:
    for(i=1; i<NumEls; ++i)
    Missing the first entry of the array out each time the function is run (first element of an array is defaulted as 0).



    Code:
    Max == *pVals;
    "warning C4553: '==' : operator has no effect; did you intend '='?"

    This is what I have so far.

    Trying to determine the problems of original problem by using the following code:
    Code:
    #include "stdafx.h"
    #include <iostream>
    using namespace std;
    
    
    
    int main()
    {
    int * pVals;
    int NumEls[5];
    pVals = NumEls; 
    *pVals = 10;
    pVals++;  *pVals = 20;
     pVals = &NumEls[2];  *pVals = 30;
     pVals = NumEls + 3;  *pVals = 40;
     pVals = NumEls;  *(pVals+4) = 50;
    
    	int i, Max = *pVals++;
    
    	for (i=0; i<5; ++i)
    cout << NumEls[i] << ", ";	
    	{
    			if(Max < *pVals++)
    			Max == *pVals;
    	}
    cout << Max;
    return Max;
    }
    Last edited by scojan; 10-16-2007 at 04:18 AM. Reason: added code

  9. #9
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> Defining ‘i = 1’ in the line: for(i=1; i<NumEls; ++i)
    Nothing "wrong" with this, it doesn't define i it initializes it. Although I would have actually defined i there: for(int i=1; i<NumEls; ++i)

    >> Missing the first entry of the array out each time the function is run
    Is it? Look again.

    >> "warning C4553: '==' : operator has no effect; did you intend '='?"
    Do you know what that means?

  10. #10
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,817
    The other issue is this:
    Code:
    if(Max < *pVals++)
        Max = *pVals;
    You check the current Max value with the value in the array and if you need to update Max you update it with the next value (not the one you just compared against) since the pointer has already been incremented by the time you reach the assignment. If the last element in the array happens to be the highest value (as an example), then you set Max to whatever value is in the next slot (an out-of-bounds memory location) which could be anything, even a really low value. You should not be doing the increment of the pointer there.
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  11. #11
    Registered User
    Join Date
    Oct 2007
    Posts
    4
    Code:
    #include "stdafx.h"
    #include <iostream>
    using namespace std;
    
    int main ()
    {
      int NumEls[5] = {15, 25, 40, 50, 24};
      int * pVals;
      pVals = NumEls;
      int max = *pVals++;
      
      
    
      for (int i = 0; i<5; i++)
        {
          if (max < *pVals)
            
              max = *pVals;
          
        }
      cout << "Maximum element in the array :   " << max;
    
      return 0;
    }
    It is giving me the maximum value as being 25, what have I done wrong. When I change these values it is always giving the second integer in the array as the maximum value.

  12. #12
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    You do still have to increment pVals at some point... Otherwise you are just getting the first value always.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  13. #13
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    Now you don't increment the pointer at all, so the only values you ever check is the first and the second.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  14. #14
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,656
    Why are you messing with pVals ?
    You increment it in some places, and fail to increment it in other places.

    Try
    Code:
    max = NumEls[0];
    for ( i = 1 ; i < 5 ; i++ ) {
      if ( max < NumEls[i] ) max = NumEls[i];
    }


    Yes you can use the pointer, and increment it with ++, but you need to be rather careful about when you actually increment it.
    Code:
          if (max < *pVals++)
              max = *pVals++;
    Trying to use ++ in either of these places for example would be wrong.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  15. #15
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Salem View Post
    Why are you messing with pVals ?
    You increment it in some places, and fail to increment it in other places.

    Try
    Code:
    max = NumEls[0];
    for ( i = 1 ; i < 5 ; i++ ) {
      if ( max < NumEls[i] ) max = NumEls[i];
    }
    I agree. And for a simple example like this, the compiler will most likely generate at least as good code for this as for the pointer example [assuming the pointer was introduced in an attempt to "make it better"]. The above is also more obvious for anyone else reading the code.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. WS_POPUP, continuation of old problem
    By blurrymadness in forum Windows Programming
    Replies: 1
    Last Post: 04-20-2007, 06:54 PM
  2. Laptop Problem
    By Boomba in forum Tech Board
    Replies: 1
    Last Post: 03-07-2006, 06:24 PM
  3. JOB: C/C++ Developer with problem solving skills
    By VoltRecruiter in forum Projects and Job Recruitment
    Replies: 1
    Last Post: 01-26-2006, 12:25 AM
  4. Replies: 5
    Last Post: 11-07-2005, 11:34 PM
  5. Sign-up Thread: Problem Solving #1
    By ygfperson in forum Contests Board
    Replies: 15
    Last Post: 01-26-2003, 02:55 AM