# Divide two numbers, then apply a custom rounding rule

If the fractional part of the number is greater than 0.6, round up; if less, round down. I need feedback about my code.

public static double DivisionMethod(double a, double b)
{
double div = a / b;
double temp = Math.Floor(div);
double fractional = div - temp;

if (fractional > 0.6)
{
return Math.Ceiling(div);
}
else
{
return Math.Floor(div);
}
}

• Did you mean "if less or equal - round down"? Jun 26, 2014 at 15:04
• Also, by round down, do you mean towards zero or towards negative infinity? Would -2.5 round to -2 (towards zero) or -3 (towards negative infinity)? Jun 26, 2014 at 15:11
• You might enjoy this question. stackoverflow.com/questions/921180/… The lesson here is that even smart people can get division completely wrong, over and over again. You are very wise to be careful about your code and get it reviewed. Jun 26, 2014 at 18:40
• @dcastro The double constant 0.6 isn't 0.6 in the mathematical sense anymore. That your equation evaluates to true is merely luck. There are similar looking equations (such as double d = 1.2d/3d; d == 0.4 that won't do so. I'm not even sure if it your example will work reproducibly across different implementations an hardware. Jun 27, 2014 at 9:44
• @dcastro: Pulling up MiscUtil.Conversion.DoubleConverter.ToExactString, 0.6d is represented as 0.59999999999999997779553950749686919152736663818359375 (as is 1.2d/2d). Mentally replace all instances of 0.6 with that horrifying number. During output, C# often prints out 0.6 (because it chops of the last few digits for display). Jun 27, 2014 at 13:27

DivisionMethod is a pretty vague/poor name for your method; any method with the word "method" in it should raise a "bad name" flag!

Also, hard-coding the threshold into the function takes away flexibility. Perhaps I'd consider an optional parameter, like this:

public static double RoundedDivision(double a, double b, double threshold = 0.6)


Okay, not sure RoundedDivision is a much better name... naming is hard!. This is where XML comments can help:

/// <summary>
/// Divides <c>a</c> by <c>b</c> and round up if quotient is
/// greater than <c>threshold</c>, round down if it's less.
/// </summary>
public static double RoundedDivision(double a, double b, double threshold = 0.6)


It's probably best to call a cat, a cat: a is the dividend and b is the divisor, so the method would be clearer with a signature like this:

/// <summary>
/// Divides <c>dividend</c> by <c>divisor</c> and round up if quotient is
/// greater than <c>threshold</c>, round down if it's less.
/// </summary>
public static double RoundedDivision(double dividend, double divisor, double threshold = 0.6)


As was mentioned in the comments, there's an ambiguity issue with the threshold: what happens if the quotient is exactly equal to the threshold?

First, you're calculating Math.Floor(div) twice. You have already stored it in temp (and why not give it a more clear name, like floor).

Second, you don't need the fractional variable:

if (div - floor > 0.6) etc.


Third, Math.Ceiling in this case is equal to floor + 1;

The rest is a matter of personal style. Some people would remove the braces and some would use a ternary operator instead of if/else, like this:

public static double DivisionMethod(double a, double b)
{
double div = a / b;
double floor = Math.Floor(div);
return div - floor > 0.6 ? floor + 1 : floor;
}

• Math.Ceiling(...) is equal to floor+1... as long as div is not an integer. If it is an integer, they're the same. Jun 26, 2014 at 16:22
• @Snowbody If div is an integer, div - floor is 0, so floor + 1 is never reached.
– Keen
Jun 26, 2014 at 16:53
• @Snowbody You're right of course. What I meant was the return value. If floor > 0.6 then you don't have to calculate Math.Ceiling Jun 26, 2014 at 16:56
• @dennis_e Rereading the post it seems that you are saying that only in that specific case, you can guarantee Math.Ceiling() == floor+1 but I didn't pick that up the first time. Jun 26, 2014 at 16:58

• Show us your test cases and their results; we can criticize them as well and show you what test cases you should be using.

• Do you find it a little bit strange that DivisionMethod(10, 7) is 1 but DivisionMethod(-10, 7) is -2 ? I would expect that changing the sign of an operand should change the sign of the output, not the sign and magnitude of the output. This is not a question about the action of the code, but rather whether the method is poorly specified to begin with. Any method that deals with division and rounding I expect the specification to say very clearly what to do in the case of negative numbers.

• Note that 0.6 cannot be represented exactly by a double; doubles are fractions where the denominator is a power of two, and three-fifths cannot be made to have a power of two on the bottom. If your intention is to represent exactly fractions which have a power of ten on the bottom, you need to use decimal, not double.

public static double DivisionMethod(double dividend, double divisor)
{
double div = dividend / divisor;
double floor = Math.Floor(div);
return div - floor <= 0.6 ? floor : floor+ 1;
}


There is more probability that div - Math.Floor(div) is smaller then and equal to 0.6. so I think it's a good practise to use

div - floor <= 0.6 ? floor : floor + 1;


div - floor > 0.6 ? floor + 1 : floor;


by this reduced the probability of adding 1 to floor.

• especially since the second way of writing it is wrong if div happens to be an integer. Jun 26, 2014 at 16:24

If you want 0.6 -> 0.0 then return Math.Ceiling(x/y - 0.6 )

If you want 0.6 -> 1.0 then return Math.Floor(x/y + 1 - 0.6)

Taking into account previous suggestions, I propose the following, which:

• Provides better names
• Handles divide by zero
• Handles negative outcomes

public static double CustomDivision(double dividend, double divisor, double threshold = 0.6)
{

// Avoid divide by zero error
if (!divisor.Equals(0.0))
{
// Perform calculations
bool isPositive = answer < 0 ? false : true;
double remainder = answer - (isPositive ? floor : ceiling);

// Which way to round depends on sign and remainder
bool roundUp = isPositive ?
(Math.Abs(remainder) > threshold ? true : false) :
(Math.Abs(remainder) > threshold ? false : true);

// Now we know which way to round
answer = roundUp ? ceiling : floor;
}