0

Working on a Java class, its making me crazy because this expression is evaluating to zero, I need it to evaluate to a double, then round it down to the nearest int. So what Im trying to get is for days to be a whole number of days, yet when I run it through java it evaluates to 0. When I run it through my calculator it evaluates to the correct value. I would love a fix and an explanation to why this what I already have isn't working.

public int getEventDays(){
     //variables
     double daysCalc; 
     int days;

     //logic
     if (getStatus().equals("filling")){
        //this is indented less to fit everything on one line, its not this way in 
        //the fractions are for unit conversion 
        daysCalc= Math.floor(((capacity-storage)/(inflow-outflow))*(43560)*(1/3600)*(1/24));
        days = (int)daysCalc; 

     }
     else if (getStatus().equals("emptying")){
        //this is indented less to fit everything
        //the fractions are for unit conversion 
        daysCalc=Math.floor(((storage-0)/(outflow-inflow))*(43560)*(1/3600)*(1/24)); 
        days = (int)daysCalc;
     }
     else{
         days = -1;
     }

     return days;


}
  • 3
    `if (getStatus() == "filling"){` You can't compare strings like that... I'd expect it will always return -1. How are you calling / how do you know it returns 0? – John3136 Jan 20 '16 at 03:02
  • 3
    `1/3600` is `0`. try `1D/3600`. 1D might have officially broken up but they can live on through our code (and `1D/24` too) – Aarjav Jan 20 '16 at 03:04
  • 1
    Although it's not causing your problem right now, [How do I compare strings in Java?](http://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java) will help you avoid an issue in the future. – resueman Jan 20 '16 at 03:07
  • @John3136 I ran the debugger on jgrasp on some test code that called the getEventDay(), when I stepped through it I saw that it evaluated to 0 – Bekah Marie Jan 20 '16 at 03:08
  • @Aarjav do you mind me asking what the purpose of the 1D? – Bekah Marie Jan 20 '16 at 03:10
  • Actually @john3136 you often CAN compare strings this way. In this case, it's the integer division not the string compare that's the problem. – Dawood ibn Kareem Jan 20 '16 at 03:13
  • @DavidWallace You can't seriously be recommending people compare strings with `==` in Java? – John3136 Jan 20 '16 at 03:15
  • 1
    No, of course not. But THAT is not Bekah's problem here. It's the integer division that needs fixing. – Dawood ibn Kareem Jan 20 '16 at 03:16
  • in java if you use two integers for dividing (`1` and `3600`) you get an integer back, which has the "decimal part" (everything after the `.` in `1.58008`) removed. `1D` in java makes the `1` a double, or a number with the decimal part and therefore the result of division is a lovely double (with `58008` :) ). Also john is right, always use `myString.equals("value")` method. it worked in this case bc of compiler optimization but it wont always be the case. (or use a switch case statement) – Aarjav Jan 20 '16 at 03:19
  • Ok awesome I changed both things and I got the correct 823 days! thanks so much everybody! – Bekah Marie Jan 20 '16 at 03:21
  • Your string comparison `getStatus() == "value"` only works now because values are in the JVM's String pool, because you're apparently assigning strings like `status = "filling"`. Try to assign the value as `status = new String("filling")` and you'll see the comparison is not working any more. As everyone suggests, use `getStatus().equals("value")`. – nolexa Jan 20 '16 at 03:29

2 Answers2

3

Change your code to this :

daysCalc = Math.floor(((storage-0)/(outflow-inflow))*(43560)*(1.0/3600)*(1.0/24));

Explanation:

The right hand expression is returning an integer value. In your case, 1/3600 is rounded to 0, similar to the case of 1/24. Now by using 1.0 instead of 1, it is giving the unrounded float value of 1/3600.

jfdoming
  • 975
  • 10
  • 25
1

Your problem is connected with the order of operations within your expression. The parentheses around 1/3600 and 1/24 cause these expressions to be evaluated first - and since each of these divisions has an expression of integer type on either side of the division, it's treated as an integer division. In other words, 1/3600 and 1/24 are both evaluated as integers, to give a result of zero. This means that your arithmetic includes a couple of multiplications by zero, which is why your result is zero.

The simplest fix is to understand that multiplying by the reciprocal of some number is the same as dividing by that number. In other words, you could simplify the calculation to

daysCalc = Math.floor( storage / ( outflow - inflow ) * 43560 / 3600 / 24 );

which will give the correct result, provided storage, outflow and inflow are not all integers.

On the other hand, if storage, outflow and inflow are all integers, then you'll need to make sure that the first division is also not treated as an integer division. You could do this by writing

daysCalc = Math.floor((double) storage / ( outflow - inflow ) * 43560 / 3600 / 24 );

which forces the division to be done with floating point arithmetic; and thereafter, each one of the divisions is done in floating point.

Dawood ibn Kareem
  • 77,785
  • 15
  • 98
  • 110