CODEWITHSUNDEEP
c++loopsfor-loopif-statement
user6436744
user6436744

Reputation:

My If Else statement is not working to provide maximum value in a vector (C++)

I am trying to return the max. value in a vector in C++, but I am constantly getting only the last value as the response (I am guessing it is because the if-else loop is somehow not doing the comparison and is just assigning the next value to "maxVal"). For example, the below code is returning 30 as the answer. What am I doing wrong? Can you please help?

Below is the code ->

#include <iostream>
#include <cstdlib>
#include <cmath>
#include <string>
#include <fstream>
#include <vector>

using namespace std;

double max (const vector<double>& myVector)   {
int n = myVector.size();
double maxVal;
for (int i=0; i<=n-1; i++) {
    if (maxVal <= myVector[i+1])  {
        maxVal = myVector[i+1];   
    }
    else {
        maxVal = myVector[i];
    }

}
return maxVal; 
}

int main() {
vector<double> testVector;
testVector.push_back(10.0);
testVector.push_back(200.0);
testVector.push_back(30.0);
cout << max(testVector);
return 0;

}

Upvotes: 1

Views: 562

Answers (4)

Amit G.
Amit G.

Reputation: 2674

(I assume that the vector is not empty)

This work:

double max(const std::vector<double>& myVector)
{
    size_t n = myVector.size();
    double maxVal = myVector[0];

    for (size_t i = 0; i < n; i++)
    {
        if (myVector[i] > maxVal)
            maxVal = myVector[i];
    }

    return maxVal;
}

int main()
{
    std::vector<double> testVector{ 10.0 , 200.0 , 30.0 };

    std::cout << max(testVector);

    return 0;
}

Keep it simple & readable: Initialize the maxVal with the 1st element of the vector, Run on all elements, & if one is bigger than maxVal, update maxVal.

Upvotes: 2

463035818_is_not_an_ai
463035818_is_not_an_ai

Reputation: 122460

Here

for (int i=0; i<=n-1; i++) {
    if (maxVal <= myVector[i+1])  {
        maxVal = myVector[i+1];   
    }
    else {
        maxVal = myVector[i];
    }
}

you change the value of maxVal on every iteration. In the end maxVal can only be either the value of the last element or an elements one past your array. Which brings me to the next problem: Valid indicees are 0 up to (including) n-1, so when you are at i==n-1 then i+1 is out of your array.

Actaully there is no need to consider the next or previous element so change it to

for (int i=0; i<n; i++) {        // loop from 0 till n-1 
    if (maxVal < myVector[i]) {  // no need to check equality
        maxVal = myVector[i];   
    }                            // no need for else
}                                // if maxVal is bigger then it 
                                 // is already the max so far

Upvotes: 0

JimmyG
JimmyG

Reputation: 131

Make things easy on yourself, use the standard algorithms.

See https://en.cppreference.com/w/cpp/algorithm/max_element

Upvotes: 0

KostasRim
KostasRim

Reputation: 2053

C++ has a rich library and I don't understand why people don't use it. Here is a two line version that finds the maximum value of a vector. Please, don't reinvent the wheel. 

#include <iostream>                                                                                                                                                                                                
#include <vector>                                                                                                                                                                                                  
#include <algorithm>                                                                                                                                                                                                

int main()                                                                                                                                                                                                         
{                                                                                                                                                                                                                  
  auto v = std::vector{4, 3, 2, 1};                                                                                                                                                                               
  std::cout << *max_element(v.cbegin(), v.cend()) << "\n";                                                                                                                                                         
}

Upvotes: 4

Related Questions