1

Can you simplify this Math.Ceiling expression

decimal total
decimal? quantity, multiplier
int? days

total = (decimal)Math.Ceiling((double)quantity.Value * (double)days.Value * (double)multiplier);

EDIT I forgot to mention that this is Silverlight code, hence all the casts into double.

Pratik
  • 11,534
  • 22
  • 69
  • 99
Jonathan Allen
  • 68,373
  • 70
  • 259
  • 447

3 Answers3

12

Why would you want to convert everything to double? I would use:

total = decimal.Ceiling(quantity.Value * days.Value * multiplier.Value);

(You could use Math.Ceiling(decimal) instead, but I feel it's clearer that it's using decimal if you use decimal.Ceiling.)

Converting everything to double, performing the arithmetic there, and then converting back to decimal is very likely to lose information. You should very, very rarely be converting between double and decimal IMO.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
0

You didn't provide a whole lot of info, but here's one idea.

You use Nullable Types (types that end with a ?), but don't seem to actually check if they are null. (There might be code you omitted). If you know they won't be null, don't use nullable types. That will get rid of all the .Value terms in your expression.

This would change your expression to:

total = (decimal)Math.Ceiling((double)quantity * (double)days * (double)multiplier);

I don't know why you're casting each multiplier to double. I'd prefer multiplying them all together before casting to double. (check carefully for loss of precision.)

This would change your expression to:

total = (decimal)Math.Ceiling((double)(quantity * days * multiplier));

Overall, that looks simpler to me, and should be just as good.
(only testing will tell for sure!)

abelenky
  • 63,815
  • 23
  • 109
  • 159
  • 1
    He doesn't provide any values for them either... which suggests to me that there's some code between the declarations and the assignment :) My guess is that there are reasons for using the nullable value types, and the OP may well be checking them elsewhere, in code which is irrelevant to the question. – Jon Skeet Jan 14 '11 at 19:49
0

How about: 

decimal total;
decimal? quantity = null, multiplier = null;
int? days = null;

total = Math.Ceiling(quantity ?? 0 * days ?? 0 * multiplier ?? 0);
Shan Plourde
  • 8,528
  • 2
  • 29
  • 42
  • I forgot to mention that I'm using Silverlight where there is no Math.Ceiling(decimal). – Jonathan Allen Jan 14 '11 at 21:54
  • Also, by the time I get to this line I already know all the values are non-null. Hence the calls to .Value. – Jonathan Allen Jan 14 '11 at 21:57
  • Silverlight also supports extension methods, that could be another angle to approach it from. – Shan Plourde Jan 14 '11 at 22:01
  • It doesn't act the same as in the question. And instead of use null-coalesce operator and zeros (which makes the expression a little bit harder to read) I would probably prefer GetValueOrDefault of Nullable<>. – AFract Jan 14 '11 at 22:04
  • Hi Richard, functionally what differs as to the functional behaviour within the question? Personally I'm agnostic to the null coalesce operator, although I think as presented it is easier to read. What would you propose to make the expression in the question more readable then? – Shan Plourde Jan 14 '11 at 22:13