Program running indefinitely

[bayan]

This code is free of errors, but for some reason it takes a lot of time to execute. It is supposed to process a mathematical expression in the form of a string and give a numerical answer but when I enter the expression it just keeps running. I left it to run for at least to minutes and even then it did not get me a result.... The string was 1*3.......

Could someone tell me what the problem is please? Any help appreciate it.

public class calculate
{
private String expression;
private double first;
public calculate(String s)
{
expression=s;
first=number();

}
public double number()
{
String num ="";
int i=1;
String temp=expression.substring(i-1,i);
while(temp.compareTo("1")==0 || temp.compareTo("2")==0 || temp.compareTo("3")==0 || temp.compareTo("4")==0 || temp.compareTo("5")==0 ||
temp.compareTo("6")==0 || temp.compareTo("7")==0 || temp.compareTo("8")==0 || temp.compareTo("9")==0 || temp.compareTo(".")==0)
{
num+=temp;
i++;
}
expression.substring(0,num.length());
if (num.indexOf(".")==-1)
num+=".0";
return Double.parseDouble(num);

}

public double evaluate()
{
if(expression.length()!=0)
{double num1=first;
double ans=0;
String operation=expression.substring(0,1);
expression=expression.substring(1);
double num2=number();
if(operation.compareTo("+")==0)
ans= (num1+num2);
if(operation.compareTo("-")==0)
ans= (num1-num2);
if(operation.compareTo("*")==0)
ans= (num1*num2);
if(operation.compareTo("/")==0)
ans= (num1/num2);
if(operation.compareTo("%")==0)
ans= (num1%num2);
return ans;
}
else
{return 0.0;}
}

public String getExpression()
{return expression;}


}

import javax.swing.JOptionPane;
public class calculator
{
public static void main(String[]args)
{
double answer = 0.0;
String s=JOptionPane.showInputDialog("Enter expression","");
calculate calc = new calculate(s);
while(calc.getExpression().length()!=0)
{answer += calc.evaluate();}
JOptionPane.showMessageDialog(null,"The answer is" + answer);

}
}

Btw could a mod tell me why I have to log in every time I navigate to a new page in the forum?: :mad:

[gcalvin]

Please use CODE tags when posting code. Your code is very difficult to read when it is not formatted properly.

public class calculate

Start your class name with an upper case letter, and make it a noun rather than a verb. This means that Calculator is a much better class name than calculate. Method names should be verbs -- calculate is not bad as a method name.

{
    private String expression;
    private double first;

    public calculate(String s)
    {
        expression = s;
        first = number();
    }

Choose a better parameter name than s -- it's meaningless.

    public double number()
    {
        String num = "";
        int i = 1;
        String temp = expression.substring(i - 1, i);
        while (temp.compareTo("1") == 0 || temp.compareTo("2") == 0 || 
                 temp.compareTo("3") == 0 || temp.compareTo("4") == 0 || 
                 temp.compareTo("5") == 0 || temp.compareTo("6") == 0 || 
                 temp.compareTo("7") == 0 || temp.compareTo("8") == 0 ||  
                 temp.compareTo("9") == 0 || temp.compareTo(".") == 0)

temp.equals("1") is much simpler than temp.compareTo("1") == 0. Better yet, replace all that code with "0123456789.".contains(temp). (Better style would be to put that string in a constant.)

        {
            num += temp;
            i++;
        }

This is where your endless loop is. You increment i, but you never get another character from your expression String (the initial substring() call is before the while statement).

        expression.substring(0, num.length());
        if (num.indexOf(".") == -1)
            num += ".0";

Don't get into this habit. If you are doing a newline after your if condition, then use curly braces too.

        return Double.parseDouble(num);
    }

    public double evaluate()
    {
        if (expression.length() != 0)
        {
            double num1 = first;
            double ans = 0;
            String operation = expression.substring(0, 1);
            expression = expression.substring(1);
            double num2 = number();
            if (operation.compareTo("+") == 0) // use .equals like we said earlier
                ans = (num1 + num2); // don't need these parens
            if (operation.compareTo("-") == 0)
                ans = (num1 - num2);
            if (operation.compareTo("*") == 0)
                ans = (num1 * num2);
            if (operation.compareTo("/") == 0)
                ans = (num1 / num2);
            if (operation.compareTo("%") == 0)
                ans = (num1 % num2);
            return ans;
        }
        else
        {return 0.0;}
    }

    public String getExpression()
    {
        return expression;
    }
}

import javax.swing.JOptionPane;

public class calculator

OK, this is why you didn't call your other class Calculator. Still, use a noun of some kind. Call it CalculatorEngine or something.

{

    public static void main(String[]args)
    {
        double answer = 0.0;
        String s = JOptionPane.showInputDialog("Enter expression","");
        calculate calc = new calculate(s);
        while (calc.getExpression().length() != 0)
        {
            answer += calc.evaluate();
        }
        JOptionPane.showMessageDialog(null, "The answer is" + answer);
    }
}

[bayan]

I inserted a statement

temp=expression.substring(i-1,i);

into the loop. I got a runtime error about invalid parameters for the substring() method. It was why I had to remove it in the first place :|

-Bayan-