CODEWITHSUNDEEP
c++
Nicholas Rawitscher
Nicholas Rawitscher

Reputation: 45

Maximum Pair Product strange results

I have been trying to learn C++ from a C# background, and I am running in to a very weird mistake in my Maximum Pair Product method.

The method works as expected with small numbers. For products of large numbers it produces a strange rounding and outputs the incorrect output.

int64_t Testing::MaximumPairProductFast(const std::vector<int>& numbers)
{ 

    int64_t largestIntA = 0;

    int64_t largestIntB = 0;

    for (int  i = 0; i < numbers.size(); i++)
    {
        // find first largest number
        if (numbers[i] > largestIntA) largestIntA =  numbers[i];
    }


    for (int i = 0; i < numbers.size(); i++)
    {
        //find second largest number
        if (numbers[i] > largestIntB && numbers[i]!= largestIntA) largestIntB = numbers[i];
    }

    return  largestIntA * largestIntB;


}

The main program

int main()
{

    Testing ch;

    std::vector<int> test{ 100000,90000 };  

    int result= ch.MaximumPairProductFast(test);

    std::cout << result << "\n";

}

The output of the computation will be 410065408 instead of 9000000000 which is the correct answer

The weird thing is when I try to print this:

std::cout << (int64_t) 100000 * 90000 << "\n";

It does give the correct result 9000000000.

But if I don't cast it to int64_t :

std::cout <<  100000 * 90000 << "\n";

The result is 410065408, which is exactly the output from my method even though I have ensured to return an int64_t type.

I have also tried this, but the outcome is the same:

int64_t Test::MaximumPairProductFast(const std::vector<int>& numbers)
{ 

    int64_t largestIntA = 0;

    int64_t largestIntB = 0;

    for (int  i = 0; i < numbers.size(); i++)
    {
        // find first largest number
        if ((int64_t) numbers[i] > largestIntA) largestIntA = (int64_t) numbers[i];
    }


    for (int i = 0; i < numbers.size(); i++)
    {
        //find second largest number
        if ((int64_t) numbers[i] > largestIntB && (int64_t) numbers[i]!= largestIntA) largestIntB = (int64_t) numbers[i];
    }


    return  largestIntA * largestIntB;

}

Am I missing an obvious detail?

Upvotes: 0

Views: 36

Answers (2)

theWiseBro
theWiseBro

Reputation: 1529

On your system int is a 32-bit integer. int64_t is 64-bit integer. The problem in your code is you are multiplying two 32-bit ints so you're getting a 32-bit int result. Your answer is overflowing. When you explicitly cast it to int64_t, the operands are now 64-bit ints and you're getting a 64-bit int result which can hold the large value and not overflow.

I'll suggest you either:

  1. use std::vector<int64_t> instead of std::vector<int>
  2. If you're worried about space, do as you did, explicitely cast it to 64-bit int and then multiply

Upvotes: 1

1201ProgramAlarm
1201ProgramAlarm

Reputation: 32727

You are truncating your result from a int64_t to an int. On a typical system these days, you'll truncate a 64 bit integer and store it in a 32 bit value.

If you compile with the warning level increased, the compiler will tell you about this problem.

The solution is to declare result with the proper type:

int64_t result = ch.MaximumPairProductFast(test);

Or, just let the compiler use the correct type with auto:

auto result = ch.MaximumPairProductFast(test);

In your second example, the constants 100000 and 90000 are integers, so the multiplication of them will be done as integers. When you case one of the values to int64_t, the multiplication will be done with 64 bit ints. This can also be expressed with 100000LL.

Upvotes: 2

Related Questions