1

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)

4
  • 5
    The currPixel / modifier expression is integer division, so if modifier is larger than currPixel then the result will be zero. Oct 30, 2014 at 10:46
  • You're assigning the result of dividing integers to a double hence 0
    – EdChum
    Oct 30, 2014 at 10:46
  • Try changing your integers to doubles. Since your result is a double the result will be false / 0.
    – Matheno
    Oct 30, 2014 at 10:46
  • Seems like it is having rounding issues
    – ha9u63a7
    Oct 30, 2014 at 10:48

5 Answers 5

3

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.

2

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.

1
  • Add the cast before the division is done. (double)(1/2) is still 0.
    – MSalters
    Oct 30, 2014 at 10:58
0

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.

3
  • well, (int) ((5.0 * 6.0) / 20.0) == (int) (1.5) == 1 AFAIK. Not much of a difference.
    – Ronald
    Oct 30, 2014 at 10:58
  • The existing code rounds the double back to int on return. There might be some direction of rounding differences for negative inputs, though.
    – MSalters
    Oct 30, 2014 at 11:00
  • What's lacking here is an exact specification of the behaviour of the function. Should non integer results be rounded or truncated? Since the return value of the function is an int, we'll have an error most of the times. The calculation above doesn't make it worse, is deterministic and fast.
    – Ronald
    Oct 30, 2014 at 11:07
0

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;
0

casting to double should fix the error.

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

see type casting rules typecasting rules in c.

Not the answer you're looking for? Browse other questions tagged or ask your own question.