Win a copy of Design for the Mind this week in the Design forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Which Code is Better - Can you help me decide?

 
Varun Chopra
Ranch Hand
Posts: 213
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have to write code for calculating tax. Tax rate is 20% for male and 18% for female members. Some options I have are below, there may be more options, what should I prefer and why?

A. Only one function:



B. 2 separate functions



C. leave logic of checking gender to client side (to the calling code)


[RP] added code tags [/RP]
[ October 03, 2008: Message edited by: Rob Prime ]
 
Rob Spoor
Sheriff
Pie
Posts: 20511
54
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Please Use Code Tags. It preserves the formatting. I've added them for you this time.

First, let me help you with some logic errors.



Use equals to check for equality. == checks if two references are pointing to the exact same object. This is rarely the case, even with strings. equals solves this for you:

or shorter

By the way, the female percentage will always be used because it's spelled "male"


Both will return 0. This is because 20/100 and 18/100 are both 0 - integer division will not return a float or double. Instead, turn one or both of the operands to a double by appending .0 to it:



Now if I were you I'd go for option three. What if youths only need to pay 15%? You don't want to add a third method (option 3). That leaves options 1 and 3, but option 1 uses strings to determine the type. As you've already made a mistake in your code, it's quite easy to have the wrong tax amounts determined. If I even passed "Mail" the method would regard me as female.


One final note: using float and double will eventually lead to rounding errors; that's just how floating point calculations work. You can see it even with a simple example:

Since your calculations are about money, you can't afford such errors. java.math.BigDecimal is a better solution then.
 
Varun Chopra
Ranch Hand
Posts: 213
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for your comments Rob.
I was not trying to be syntactically correct or accuracy wise optimal. My intention was to get pros/cons of different approaches.

As you have mentioned approach 3 is better, one question I have regarding that is "won't that increase code duplication at client side?" Especially if clients do not decide tax rates, instead rates are read from a central repository (like Federal Tax Rates)?
 
Rob Spoor
Sheriff
Pie
Posts: 20511
54
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Then you add an extra method over it that determines the rates for you.

If this rate can be retrieved from a database for instance, there is no need for the if statement. In pseudo code:

Of course if the rate is hard coded somewhere then you still need an if statement, but again: don't let the user fill in an arbitrary string as his "rate type" (don't call it gender, see my youth example). Either retrieve this from somewhere, or use an enum so the choices are limited.

Actually, an enum would be excellent:

No more mistyping, no more if statements, just use the enum:

When calling, you are limited to RateType.Male, RateType.Female and RateType.Youth (and null). Any other option is a compile time error.
[ October 03, 2008: Message edited by: Rob Prime ]
 
Varun Chopra
Ranch Hand
Posts: 213
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks again. Yes that looks good :-)
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Remember that enums can also have constructors. I'd probably rather type

 
Thomas Thevis
Ranch Hand
Posts: 87
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rob and Ilja,

is there a special reason to use 'abstract' enums?

Just curious,
Thomas
 
Rob Spoor
Sheriff
Pie
Posts: 20511
54
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Not really. If I create any class with an abstract method, I automatically make that class abstract. Guess it applies to enums as well
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And I just copied Rob's code. Didn't even notice it.
 
Thomas Thevis
Ranch Hand
Posts: 87
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If I create any class with an abstract method, I automatically make that class abstract.

Seems reasonable
Thank you guys for clarification.

Thomas
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic