CODEWITHSUNDEEP
c++divisioninteger-division
Halfpint
Halfpint

Reputation: 4079

Why is my double or int value is always 0 after division?

I'm fairly new to C++ and I'm experiencing some strange behaviour from a percentage increase method I am writing for some image editing software.

What I want to do is give the R G or B value of the current pixel and divide it by some modifier, then multiply it by the new value to return the percentage increase, fairly easy concept.

However, whenever I run my debugger, the return value is always 0, I thought this may be because I was trying to do operations which give negative numbers on an integer (or maybe a divide by zero could occur?), so I tried to use a double to store the output of the computation, however I've had no luck.

The code I'm struggling with is below:

int Sliders::getPercentageIncrease(int currPixel, int newValue, int modifier)
{
    // calculate return value
    double returnVal = (currPixel / modifier) * newValue;

    // Check we are returning a positive integer
    if(returnVal >= 0)
        return (int)returnVal;

    // Return a negative integer value
    return (int)(0 - returnVal);
}

What am I doing wrong here?

NOTE: I have checked values, of inputs in my debugger and I get stuff like:

currPixel = 30
newValue = 119
modifier = 200

From this I would expect an output of 18 (I am not concerned with returning decimal figures)

Upvotes: 1

Views: 4511

Answers (5)

Robert
Robert

Reputation: 1343

Since all three parameters are integer the result of the calculation

double returnVal = (currPixel / modifier) * newValue;

will always be truncated. Add cast to (double) and the result should be fine. Simply:

double returnVal = ((double)currPixel / modifier) * newValue;

If you only set a cast before the bracket the result of the division stays an integer.

Upvotes: 2

Baldrickk
Baldrickk

Reputation: 4409

Your current calculation only involves integers and so will be affected by integer division (which truncates the result to the nearest integer value). 

(currPixel / modifier) * newValue
     |           |
      ---------------integer division e.g. 10/3 = 3, not 3.333

The result is then cast to double, but the accuracy is lost before this point.

Consider the following:

#include <iostream>
using namespace std;

int main() {
    int val1 = 10;
    int val2 = 7;
    int val3 = 9;

    double outval1 = (val1 / val2) * val3;
    double outval2 = ((double)val1 / val2) * val3;
    cout << "without cast: " << outval1 << "\nwith    cast: "<< outval2 << std::endl;

    return 0;
}

The output of this is:

without cast: 9
with    cast: 12.8571

See it here

Note that the cast has to be applied in the right place:

(double)(val1 / val2) * val3 == 9.0      //casts result of (val1/val2) after integer division
(val1 / val2) * (double)val3 == 9.0      //promotes result of (val1/val2) after integer division
((double)val1 / val2) * val3 == 12.8571  //promotes val2 before division
(val1 / (double)val2) * val3 == 12.8571  //promotes val1 before division

Due to promotion of the other operands, if in doubt you can just cast everything and the resulting code will be the same:

((double)val1 / (double)val2) * (double)val3 == 12.8571

It is a little more verbose though.

Upvotes: 3

Vulcan
Vulcan

Reputation: 36

casting to double should fix the error. 

double returnVal =  (double ) (currPixel) / (modifier) * newValue;

see type casting rules typecasting rules in c.

Upvotes: 0

Ajay
Ajay

Reputation: 18411

Do this:

// calculate return value
double returnVal = (static_cast<double>(currPixel) / modifier) * newValue;

Or this:

double returnVal = (currPixel / static_cast<double>(modifier)) * newValue;

As you know that operator / will be performed first, and then the operator *. I have typecasted one of the operands of / to double, and hence division will be performed double. Now, left operand of * would be double (since / produced double), and the multiplication would be performed double also. For clarity and correctness, you may write:

double returnVal = (static_cast<double>(currPixel) / static_cast<double>(modifier)) * static_cast<double>(newValue);

Or simply:

double returnVal = (double(currPixel) / (double)modifier) * (double)newValue;

But, following is WRONG:

double returnVal = (double)(currPixel / modifier) * /*(double)*/ newValue;

Since the division would be performed int only! It is like:

double x = 10/3;

Where you need (either):

double x = 10.0/3;
double x = 10/3.0;
double x = (double)10/3;

Upvotes: 0

Ronald
Ronald

Reputation: 2882

As long as all values are in a range, let me say, less than 1000 and greater (or equal) than 0, which is common on colour values, do something like

int returnVal = (currPixel * newValue) / modifier

No need for doubles; it will even speed up the code. Needless to say, modifiershould not be zero.

Upvotes: 0

Related Questions