patternjavaMinor
Income Tax Calculator Simulator via J2ME
Viewed 0 times
j2meincometaxviasimulatorcalculator
Problem
I've been imposed with the task of implementing an income tax calculator via J2ME. I have managed to devise the following program solely by myself, however the person who decreed me to carry out this task deems my code to be atrocious and says it can be optimized and altered to be better, asking me to figure it out. I'm absolutely baffled and stunted and don't know what to do.
```
public class HelloMidlet extends MIDlet implements CommandListener
{
TextField gs,ma,pf,age,loan,years,interest,emif;
ChoiceGroup cal_sel;
// ChoiceGroup gender;
Display display;
Command add,OK,Exit,Exiti,back,backi,backt,backe,sub,div,mul,History,proc,proci,proce;
int result;
private int currentIndex;
Form f1,f2,f3,fi;
String message;
int page,pgs,pma,ppf,ded,eloan,eyears,months,i;
float einterest;
double tax,roi;
double emi=0.0d;
public HelloMidlet()
{
gs = new TextField("Gross Salary", "", 10, TextField.NUMERIC);
//all = new TextField("Allowances", "", 10, TextField.NUMERIC);
ma = new TextField("Medical Allowance", "", 10, TextField.NUMERIC);
pf = new TextField("PF", "", 10, TextField.NUMERIC);
age = new TextField("Age", "", 3, TextField.NUMERIC);
loan = new TextField("Loan", "", 10, TextField.NUMERIC);
years = new TextField("Years", "", 2, TextField.NUMERIC);
interest = new TextField("Interest Rate", "", 3, TextField.NUMERIC);
emif = new TextField("EMI", "", 15, TextField.UNEDITABLE);
backi = new Command("BACK", Command.BACK,1);
backt = new Command("BACK", Command.BACK,1);
backe = new Command("BACK", Command.BACK,1);
// add = new Command("ADD", Command.OK, 2);
Exit=new Command("EXIT", Command.EXIT, 7);
Exiti=new Command("EXIT", Command.EXIT, 1);
proc = new Command("Process", Command.SCREEN,1);
proci = new Command("Process", Command.SCREEN,1);
proce = new Command("Process", Command.SCREEN,1);
back = new Command("BACK", Command.SCREEN,1);
cal_sel = new ChoiceGroup("Select calculator", Choice.EXCLUSIVE);
currentIndex = cal_sel.append("Income Tax Calculator",null);
cal_sel.append("EM
```
public class HelloMidlet extends MIDlet implements CommandListener
{
TextField gs,ma,pf,age,loan,years,interest,emif;
ChoiceGroup cal_sel;
// ChoiceGroup gender;
Display display;
Command add,OK,Exit,Exiti,back,backi,backt,backe,sub,div,mul,History,proc,proci,proce;
int result;
private int currentIndex;
Form f1,f2,f3,fi;
String message;
int page,pgs,pma,ppf,ded,eloan,eyears,months,i;
float einterest;
double tax,roi;
double emi=0.0d;
public HelloMidlet()
{
gs = new TextField("Gross Salary", "", 10, TextField.NUMERIC);
//all = new TextField("Allowances", "", 10, TextField.NUMERIC);
ma = new TextField("Medical Allowance", "", 10, TextField.NUMERIC);
pf = new TextField("PF", "", 10, TextField.NUMERIC);
age = new TextField("Age", "", 3, TextField.NUMERIC);
loan = new TextField("Loan", "", 10, TextField.NUMERIC);
years = new TextField("Years", "", 2, TextField.NUMERIC);
interest = new TextField("Interest Rate", "", 3, TextField.NUMERIC);
emif = new TextField("EMI", "", 15, TextField.UNEDITABLE);
backi = new Command("BACK", Command.BACK,1);
backt = new Command("BACK", Command.BACK,1);
backe = new Command("BACK", Command.BACK,1);
// add = new Command("ADD", Command.OK, 2);
Exit=new Command("EXIT", Command.EXIT, 7);
Exiti=new Command("EXIT", Command.EXIT, 1);
proc = new Command("Process", Command.SCREEN,1);
proci = new Command("Process", Command.SCREEN,1);
proce = new Command("Process", Command.SCREEN,1);
back = new Command("BACK", Command.SCREEN,1);
cal_sel = new ChoiceGroup("Select calculator", Choice.EXCLUSIVE);
currentIndex = cal_sel.append("Income Tax Calculator",null);
cal_sel.append("EM
Solution
There is a lot in this code that doesn't follow general conventions, and it honestly is quite hard to read. But if you follow some of the points below, I think it shouldn't be too hard to make it a lot better quickly.
Formating
Your formating makes your code quite hard to read. Especially your indentation and your use of spaces (or lack thereof). If you paste your code in any IDE, it will fix this for you (for example, in Netbeans just press
It is also not common practice to declare multiple variables on one line. It makes it easy to overlook a variable. It also makes it easy (as in your case) to declare way too many variables (at the wrong level), because it doesn't look like that much.
Naming
Variable names should give a reader a good idea what a variable does. But I wouldn't know what the difference between
Also, try to avoid short variable names, especially at a class level (
If it is a technical term, short(ish) variable names might be ok (as for example
The same goes for method names.
Also, it is customary to use camelCase in Java, not underscores (
Fields
You have way too many fields (class level variables) for such a small class. Try to make the scope of a variable as small as possible.
For example, the Command fields are really only used in
And loop variables should be declared inside the loop:
Overly long method
First of all, create new methods for each command (so you have an
Then, create new methods for taxable and non-taxable pgs.
Also, try to separate the parsing of input and the outputing of data from the calculation. They are different things and should not happen in the same method.
Magic Numbers
Don't hardcode numbers, store them in a static final field. That way, you can give them a nice name, and you can reuse them (for example,
Command
I don't have the
Classes
Your current class does everything. It builds the GUI, it performs the tax calculation, etc. Try to create more classes, each for its specific purpose.
Formating
Your formating makes your code quite hard to read. Especially your indentation and your use of spaces (or lack thereof). If you paste your code in any IDE, it will fix this for you (for example, in Netbeans just press
Alt + Shift + f).It is also not common practice to declare multiple variables on one line. It makes it easy to overlook a variable. It also makes it easy (as in your case) to declare way too many variables (at the wrong level), because it doesn't look like that much.
Naming
Variable names should give a reader a good idea what a variable does. But I wouldn't know what the difference between
Exit and Exiti or back, backi, backt, and backe is without looking deep into your code.Also, try to avoid short variable names, especially at a class level (
gs, ma, pf, emif, f1, f2, f3, fi, i, ppf, and so on). But also for method parameters (c, d) and inside methods (bl1, bl2, bl3, bl4) expressive variable names can go a long way to make code readable.If it is a technical term, short(ish) variable names might be ok (as for example
roi), but I would add a comment, for people who are not that deep into the matter.The same goes for method names.
itc and emi are not all that clear (at least to me). If they are technical terms, write a comment for the method.Also, it is customary to use camelCase in Java, not underscores (
cal_sel -> calSel), and variables should start with a lowercase letter (History -> history).Fields
You have way too many fields (class level variables) for such a small class. Try to make the scope of a variable as small as possible.
For example, the Command fields are really only used in
commandAction (as are a lot of other fields).emi, months, page, pma, ppf, message are further example of variables that should not be declared at a class level, but right where you need them. And loop variables should be declared inside the loop:
for(int i = 0; i < months; i++), not at a class level.Overly long method
commandAction is way too long, and thus makes it very hard to read.First of all, create new methods for each command (so you have an
exitAction, an proceAction and procAction action (although don't name them that, I am just unsure what proce is), and so on).Then, create new methods for taxable and non-taxable pgs.
Also, try to separate the parsing of input and the outputing of data from the calculation. They are different things and should not happen in the same method.
Magic Numbers
Don't hardcode numbers, store them in a static final field. That way, you can give them a nice name, and you can reuse them (for example,
200000 appears more than once. But I wouldn't know if this is accidental, or if it the same meaning behind the 200000).Command
I don't have the
Command class, but right now, you are just creating instances of it to compare it to the input. Instead, I would store them somewhere (for example as static final fields in the Command class).Classes
Your current class does everything. It builds the GUI, it performs the tax calculation, etc. Try to create more classes, each for its specific purpose.
Context
StackExchange Code Review Q#63789, answer score: 3
Revisions (0)
No revisions yet.