3
\$\begingroup\$

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("EMI Calculator",null);
cal_sel.setSelectedIndex(currentIndex, true);
 }

public void startApp() 
{
fi=new Form("CALCULATORS");
f1=new Form("Income Tax Calculator");
f2=new Form("Tax");
f3=new Form("Home Loan EMI Calculator");

display =Display.getDisplay(this);

fi.append(cal_sel);
fi.addCommand(proci);
fi.addCommand(Exiti);
fi.setCommandListener(this);

f1.append(gs);
f1.append(ma);
f1.append(pf);
f1.append(age);
//f1.addCommand(History);
// f1.addCommand(add);
// f1.addCommand(OK);
//f1.addCommand(Exit);
f1.addCommand(backi);
// f1.addCommand(sub);
// f1.addCommand(div);
// f1.addCommand(mul);
f1.addCommand(proc);
// f1.append(gender);
f1.setCommandListener(this);

f2.addCommand(backt);
f2.setCommandListener(this);


f3.append(loan);
f3.append(years);
f3.append(interest);
//f3.append(emif);
f3.addCommand(proce);
f3.addCommand(backe);
f3.setCommandListener(this);

initial_screen();

}

public void pauseApp() {
}

public void destroyApp(boolean unconditional) 
{
notifyDestroyed();

 }

 public void initial_screen(){

//f1.setItemStateListener(this);
display.setCurrent(fi);
}

public void itc(){
display.setCurrent(f1);
}


 public void emi(){
display.setCurrent(f3);
 }
 public void commandAction(Command c,Displayable d)
 {
if(c==Exit || c==Exiti)
{
    destroyApp(true);
}
else if (c == proci)
{
    currentIndex = cal_sel.getSelectedIndex();
    String message = new String(cal_sel.getString(currentIndex));
    if( message.equals("Income Tax Calculator"))
    {
    gs.setString("");
    age.setString("");
    ma.setString("");
    pf.setString("");
    itc();
    }
    else if(message.equals("EMI Calculator"))
    {
    loan.setString("");
    years.setString("");
    interest.setString("");
    emi();
    }
}
else if (c == backi || c== backe)
{
    initial_screen();
    //emif.setString("");
}else if (c == backt)
{
    f2.deleteAll();
        gs.setString("");
        age.setString("");
        ma.setString("");
        pf.setString("");
    itc();
}
else if (c == proc)
{
        String bl1,bl2,bl3,bl4;
    pgs=Integer.parseInt(gs.getString());

    bl1=gs.getString();
    bl2=age.getString();
    bl3=ma.getString();
    bl4=pf.getString();

    if(pgs>200000)              //Taxable
    {
        tax=0.0;
        page=Integer.parseInt(age.getString());
        pma=Integer.parseInt(ma.getString());
        ppf=Integer.parseInt(pf.getString());

        message="Gross salary="+pgs+"\n";
        f2.append(message);

        message="Medical Allowance="+pma+"\n";
        f2.append(message);

        message="PF="+ppf;
        f2.append(message);

        if(pma>15000)
        pgs=pgs-15000;
        else
        pgs=pgs-pma;

        if(ppf>100000)
        pgs=pgs-100000;
        else 
        pgs=pgs-ppf;
            if(page<60)
            {
            message="Total taxable Amount="+(pgs-=200000);
            f2.append(message);

            if(pgs>200000 && pgs<=500000)
                tax=0.1*pgs;
            else if(pgs>500000 && pgs<1000000)
                {
                tax=30000+(pgs-50000)*0.2;
                }
            else if(pgs>1000000)
                {
                tax=130000+(pgs-1000000)*0.3;
                }


            }
            else if(page>=60 && page<80)
            {
            message="Total taxable Amount="+(pgs-=250000);
            f2.append(message);

            if(pgs>250000 && pgs<=500000)
                tax=0.1*pgs;
            else if(pgs>500000 && pgs<1000000)
                {
                tax=25000+(pgs-50000)*0.2;
                }
            else if(pgs>1000000)
                {
                tax=125000+(pgs-1000000)*0.3;
                }
            }
            else 
            {
            message="Total taxable Amount="+(pgs-=500000);
            f2.append(message);

            if(pgs>500000 && pgs<1000000)
                {
                tax=(pgs-50000)*0.2;
                }
            else if(pgs>1000000)
                {
                tax=100000+(pgs-1000000)*0.3;
                }
            }

        message="Total Tax="+tax;
        f2.append(message);

        message="Educational Cess@2%="+(tax*0.02);
        f2.append(message);

        message="Secondary and Higher Educational Cess@1%="+(tax*0.01);
        f2.append(message);

        tax+=(tax*0.02)+(tax*0.01);

        message="Final Payable  Tax="+tax;
        f2.append(message);
        display.setCurrent(f2);
    }
    else                        //Non Taxable
    {
        message="Your income is non taxable";
        f2.append(message);
        display.setCurrent(f2);
    }
}
else if(c == proce)
{
    emi=0.0;

    eloan=Integer.parseInt(loan.getString());
    einterest=Float.parseFloat(interest.getString());
    eyears=Integer.parseInt(years.getString());
    months=0;
    months=eyears*12;
    double temp=(1.0d+(einterest/(12.0*100.0d)));
    double original_temp=temp;

    for(i=0;i<months;i++)
    {
    temp*=original_temp;
    }

    emi=(eloan*(einterest/(12.0*100.0d)))*(temp);

    emi=emi/(temp-1.0);
    message=emi+"";
    //emif.setString(message);
    f3.append("EMI="+message);
    loan.setString("");
    years.setString("");
    interest.setString("");
}
}
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

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 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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.