2
\$\begingroup\$

So I feel like I'm a good coder, but that's from my point of view. Can you take a look at my code and give me some tips or criticize me on my code. Or give me some tips or things to add or make things smaller.

console class

        public class console {

    public static void main(String[] args) {

//makes the window/console
window console=new window();

}

}

window class

        import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

//the main terminal
public class window extends JFrame{
//variables
static JTextArea window;
JTextField input;

//main window for commands and shit
public window(){
//super because it is
super("my Java Console");

//able to call c.parse
final findCommand c=new findCommand();

//the input is set and shit
input = new JTextField();
  input.addActionListener(
     new ActionListener(){
        public void actionPerformed(ActionEvent event){
            input.setText("");
           c.parse(event.getActionCommand());
        }
     }
  );
  add(input, BorderLayout.NORTH);


  //this window is set and shit
  window = new JTextArea();
  window.setEditable(false);
  add(new JScrollPane(window));
  setSize(300,150);
  setVisible(true);
}

}

displayInWindow class

        public class displayInWindow {

   //display stuff in the window in the window class
public static void display(String s){
   //the JTextArea is the window and one long continues sentence but with \n
    window.window.append(s+"\n");
}

}

findCommand class

    public class findCommand {

//finds out what command to run
public void parse(String s){
//deletes spaces before and after
s=s.trim();


//splits into an array
String[] cmd=s.split(" ");

//depending on the length of the string array to find how many parameters it has
if(cmd.length==2){
    switch(cmd[0].toLowerCase()){
    //the print command
    case "print":displayInWindow.display(cmd[1]);break;
    //gets a variable that was set
    case "get":displayInWindow.display(basicCommands.get(cmd[1]));
    }


}else if(cmd.length==3){

    //switch is set to the first one and set to lowercase
    switch(cmd[0].toLowerCase()){
    //add
    case "add":displayInWindow.display(basicCommands.add(cmd[1], cmd[2]));break;
    //subtract
    case "subtract":displayInWindow.display(basicCommands.subtract(cmd[1], cmd[2]));break;
    //multiply
    case "multiply":displayInWindow.display(basicCommands.multiply(cmd[1],cmd[2]));break;
    //divide
    case "divide":displayInWindow.display(basicCommands.divide(cmd[1],cmd[2]));break;
    //remainder
    case "remainder":displayInWindow.display(basicCommands.remainder(cmd[1], cmd[2]));break;
    //power
    case "power":displayInWindow.display(basicCommands.power(cmd[1], cmd[2]));break;
    //set
    case "set":displayInWindow.display(basicCommands.set(cmd[1],cmd[2]));break;

    }

    //switch is set to the middle parameter and no need to set it to lowercase
    switch(cmd[1]){
    //adds
    case "+":displayInWindow.display(basicCommands.add(cmd[0], cmd[2]));break;
    //subtracts
    case "-":displayInWindow.display(basicCommands.subtract(cmd[0], cmd[2]));break;
    //multiplys
    case "*":displayInWindow.display(basicCommands.multiply(cmd[0],cmd[2]));break;
    //divides
    case "/":displayInWindow.display(basicCommands.divide(cmd[0],cmd[2]));break;
    //finds the remainder
    case "%":displayInWindow.display(basicCommands.remainder(cmd[0], cmd[2]));break;
    //set
    case "=":displayInWindow.display(basicCommands.set(cmd[0],cmd[2]));break;
    }

}else{
    if(basicCommands.variables.containsKey(s)){
        //if what typed in is a variable
        displayInWindow.display(s+" is "+basicCommands.variables.get(s) );
    }else{
        //if it is just random typing it displays it
        displayInWindow.display(cmd[0]);
    }

}

}

basicCommands class

import java.util.Hashtable;

public class basicCommands {

//hashtable for set and get
public static Hashtable variables =new Hashtable();

//adds 2 variables
public static String add(String num1,String num2){
    try{
        try{
            if(num1.contains(".")||num2.contains(".")){
                return(num1+" + "+num2+" = "+Double.toString(Double.parseDouble(num1)+Double.parseDouble(num2)));
            }else{
                return(num1+" + "+num2+" = "+Integer.toString(Integer.parseInt(num1)+Integer.parseInt(num2)));
            }
            }catch(Exception e){
                if(variables.get(num1).toString().contains(".")||variables.get(num2).toString().contains(".")){
                    return(num1+" + "+num2+" = "+num2+Double.toString(Double.parseDouble(variables.get(num1).toString())+Double.parseDouble(variables.get(num2).toString())));
                }else{
                    return(num1+" + "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())+Integer.parseInt(variables.get(num2).toString())));
                }
            }
    }catch(Exception e){
        if(variables.containsKey(num1)){
            if(variables.get(num1).toString().contains(".")||num2.contains(".")){
                return(num1+" + "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())+Double.parseDouble(num2)));
            }else{
                return(num1+" + "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())+Integer.parseInt(num2)));
            }
        }else{
            if(num1.contains(".")||variables.get(num2).toString().contains(".")){
                return(num1+" + "+num2+" = "+Double.toString(Double.parseDouble(num1))+Double.parseDouble(variables.get(num2).toString()));
            }else{
                return(num1+" + "+num2+" = "+Integer.toString(Integer.parseInt(num1)+Integer.parseInt(variables.get(num2).toString())));
            }
        }
    }






}

//subtracts 2 variables
public static String subtract(String num1,String num2){
    try{
        try{
            if(num1.contains(".")||num2.contains(".")){
                return(num1+" - "+num2+" = "+Double.toString(Double.parseDouble(num1)-Double.parseDouble(num2)));
            }else{
                return(num1+" - "+num2+" = "+Integer.toString(Integer.parseInt(num1)-Integer.parseInt(num2)));
            }
            }catch(Exception e){
                if(variables.get(num1).toString().contains(".")||variables.get(num2).toString().contains(".")){
                    return(num1+" - "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())-Double.parseDouble(variables.get(num2).toString())));
                }else{
                    return(num1+" - "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())-Integer.parseInt(variables.get(num2).toString())));
                }
            }
    }catch(Exception e){
        if(variables.containsKey(num1)){
            if(variables.get(num1).toString().contains(".")||num2.contains(".")){
                return(num1+" - "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())-Double.parseDouble(num2)));
            }else{
                return(num1+" - "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())-Integer.parseInt(num2)));
            }
        }else{
            if(num1.contains(".")||variables.get(num2).toString().contains(".")){
                return(num1+" - "+num2+" = "+Double.toString(Double.parseDouble(num1)-Double.parseDouble(variables.get(num2).toString())));
            }else{
                return(num1+" - "+num2+" = "+Integer.toString(Integer.parseInt(num1)-Integer.parseInt(variables.get(num2).toString())));
            }
        }
    }




}

//multiplys 2 variables
public static String multiply(String num1,String num2){
    try{
        try{
            if(num1.contains(".")||num2.contains(".")){
                return(num1+" * "+num2+" = "+Double.toString(Double.parseDouble(num1)*Double.parseDouble(num2)));
            }else{
                return(num1+" * "+num2+" = "+Integer.toString(Integer.parseInt(num1)*Integer.parseInt(num2)));
            }
            }catch(Exception e){
                if(variables.get(num1).toString().contains(".")||variables.get(num2).toString().contains(".")){
                    return(num1+" * "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())*Double.parseDouble(variables.get(num2).toString())));
                }else{
                    return(num1+" * "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())*Integer.parseInt(variables.get(num2).toString())));
                }
            }
    }catch(Exception e){
        if(variables.containsKey(num1)){
            if(variables.get(num1).toString().contains(".")||num2.contains(".")){
                return(num1+" * "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())*Double.parseDouble(num2)));
            }else{
                return(num1+" * "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())*Integer.parseInt(num2)));
            }
        }else{
            if(num1.contains(".")||variables.get(num2).toString().contains(".")){
                return(num1+" * "+num2+" = "+Double.toString(Double.parseDouble(num1)*Double.parseDouble(variables.get(num2).toString())));
            }else{
                return(num1+" * "+num2+" = "+Integer.toString(Integer.parseInt(num1)*Integer.parseInt(variables.get(num2).toString())));
            }
        }
    }



}

//divides 2 variables
public static String divide(String num1,String num2){
    try{
        try{
            if(num1.contains(".")||num2.contains(".")){
                return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(num1)/Double.parseDouble(num2)));
            }else{
                if(Double.parseDouble(num1)%Double.parseDouble(num2)==0){
                    return(num1+" / "+num2+" = "+Integer.toString(Integer.parseInt(num1)/Integer.parseInt(num2)));
                }else{
                    return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(num1)/Double.parseDouble(num2)));
                }

            }
            }catch(Exception e){
                if(variables.get(num1).toString().contains(".")||variables.get(num2).toString().contains(".")){
                    return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())/Double.parseDouble(variables.get(num2).toString())));
                }else{
                    if(Double.parseDouble(variables.get(num1).toString())%Double.parseDouble(variables.get(num2).toString())==0){
                        return(num1+" / "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())/Integer.parseInt(variables.get(num2).toString())));
                    }else{
                        return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())/Double.parseDouble(variables.get(num2).toString())));
                    }
                }
            }
    }catch(Exception e){
        if(variables.containsKey(num1)){
            if(variables.get(num1).toString().contains(".")||num2.contains(".")){
                return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())/Double.parseDouble(num2)));
            }else{
                if(Double.parseDouble(variables.get(num1).toString())%Double.parseDouble(num2)==0){
                    return(num1+" / "+num2+" = "+Integer.toString(Integer.parseInt(variables.get(num1).toString())/Integer.parseInt(num2)));
                }else{
                    return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(variables.get(num1).toString())/Double.parseDouble(num2)));
                }

            }
        }else{
            if(num1.contains(".")||variables.get(num2).toString().contains(".")){
                return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(num1)/Double.parseDouble(variables.get(num2).toString())));
            }else{
                if(Double.parseDouble(num1)%Double.parseDouble(variables.get(num2).toString())==0){
                    return(num1+" / "+num2+" = "+Integer.toString(Integer.parseInt(num1)/Integer.parseInt(variables.get(num2).toString())));
                }else{
                    return(num1+" / "+num2+" = "+Double.toString(Double.parseDouble(num1)/Double.parseDouble(variables.get(num2).toString())));
                }
            }
        }
    }



}

//finds the remainder
public static String remainder(String num1,String num2){

    try{
        try{
            if(Double.parseDouble(num1)%Double.parseDouble(num2)==0){

                return(num1+" % "+num2+" = "+Integer.toString(0));
            }else{
                return(num1+" % "+num2+" = "+Integer.toString((int) (Double.parseDouble(num1)%Double.parseDouble(num2))));
            }
        }catch(Exception e){
            if(Double.parseDouble(variables.get(num1).toString())%Double.parseDouble(variables.get(num2).toString())==0){

                return(num1+" % "+num2+" = "+Integer.toString(0));
            }else{
                return(num1+" % "+num2+" = "+Integer.toString((int) (Double.parseDouble(variables.get(num1).toString())%Double.parseDouble(variables.get(num2).toString()))));
            }
        }
    }catch(Exception e){
        if(variables.containsKey(num1)){
            if(Double.parseDouble(variables.get(num1).toString())%Double.parseDouble(num2)==0){
                return(num1+" % "+num2+" = "+Integer.toString(0));
            }else{
                return(num1+" % "+num2+" = "+Integer.toString((int)(Double.parseDouble(variables.get(num1).toString())%Double.parseDouble(num2))));
            }
        }else{
            if(Double.parseDouble(num1)%Double.parseDouble(variables.get(num2).toString())==0){
                return(num1+" % "+num2+" = "+Integer.toString(0));
            }else{
                return(num1+" % "+num2+" = "+Integer.toString((int)(Double.parseDouble(num1)%Double.parseDouble(variables.get(num2).toString()))));
            }
        }
    }






}

//powers the first one to the second one
public static String power(String num1,String num2){
    try{
        try{
            if(Math.floor(Math.pow(Double.parseDouble(num1), Double.parseDouble(num2)))==Math.pow(Double.parseDouble(num1), Double.parseDouble(num2))){
                return(num1+" ^ "+num2+" = "+Integer.toString((int)(Math.pow(Double.parseDouble(num1), Double.parseDouble(num2)))));
            }else{
                return(num1+" ^ "+num2+" = "+Double.toString(Math.pow(Double.parseDouble(num1), Double.parseDouble(num2))));
            }
        }catch(Exception e){
            if(Math.floor(Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(variables.get(num2).toString())))==Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(variables.get(num2).toString()))){
                return(num1+" ^ "+num2+" = "+Integer.toString((int)(Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(variables.get(num2).toString())))));
            }else{
                return(num1+" ^ "+num2+" = "+Double.toString(Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(variables.get(num2).toString()))));
            }
        }
    }catch(Exception e){
        if(variables.containsKey(num1)){
            if(Math.floor(Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(num2)))==Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(num2))){
                return(num1+" ^ "+num2+" = "+Integer.toString((int)(Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(num2)))));
            }else{
                return(num1+" ^ "+num2+" = "+Double.toString(Math.pow(Double.parseDouble(variables.get(num1).toString()), Double.parseDouble(num2))));
            }
        }else{
            if(Math.floor(Math.pow(Double.parseDouble(num1), Double.parseDouble(variables.get(num2).toString())))==Math.pow(Double.parseDouble(num1), Double.parseDouble(variables.get(num2).toString()))){
                return(num1+" ^ "+num2+" = "+Integer.toString((int)(Math.pow(Double.parseDouble(num1), Double.parseDouble(variables.get(num2).toString())))));
            }else{
                return(num1+" ^ "+num2+" = "+Double.toString(Math.pow(Double.parseDouble(num1), Double.parseDouble(variables.get(num2).toString()))));
            }
        }
    }




}

//set something in the hashtable
public static String set(String num1,String num2){
    variables.put(num1, num2);
    return(num1+" was set to "+num2);
}

//gets something in a hashtable
public static String get(String num1){
    try{
        return (variables.get(num1).toString());
    }catch(NullPointerException e){
        return(num1+" was not set to anything.");
    }

}


}
\$\endgroup\$
2
  • 5
    \$\begingroup\$ Consistent spacing and indenting will help you and the reviewers. \$\endgroup\$
    – Inkbug
    Commented Jul 30, 2012 at 5:24
  • 2
    \$\begingroup\$ Look at Joshua Bloch book, and compare your work and his advices, about switch, StringBulder, catch use, control and other professional practices \$\endgroup\$
    – cl-r
    Commented Jul 30, 2012 at 7:01

2 Answers 2

5
\$\begingroup\$

Some tips:

  • In Java there is a convention - names of classes start with uppercase letter
  • Instance variables like JTextArea etc. should be private and not static. Static fields should be used very rarely.
  • Every Swing component should be created and changed on EDT. Starting point for Swing applications usually looks like this:

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                MyFrame frame = new MyFrame();
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setVisible(true);
            }
        });
    }
  • Use proper variable names: if I see variable 'window' I think that it's of type JFrame, JDialog, JWindow etc. for JTextField it can be something like 'commandTF', 'commandField', etc.
  • Use IDE and use formatter (Eclipse, Netbeans, etc.)
  • displayInWindow is unnecessary - too little functionality. You'll end up with so called "spaghetti-code"
  • Usually if you have many switch statements with many close in concept cases - replace it with polymorphism (enums are also useful)
\$\endgroup\$
4
\$\begingroup\$

@Xeon mentioned some good point, and here are some other:

  1. Comment like this does not help too much:

    //splits into an array
    String[] cmd=s.split(" ");
    

    If somebody knows Java they understand the code, so the comment is only duplication and noise. Further reading: Clean Code by Robert C. Martin, Chapter 4: Comments

  2. In your code String.split does not handle well if users put two or more spaces between the arguments. I'd use Guava's Splitter here. It could trimResults and omitEmptyStrings.

  3. I'm not completely sure if it could help or not, but I'd check Apache Commons CLI and try to parse the arguments with this library. The hasArgs(int num) method looks promising. Further reading: Effective Java, 2nd Edition, Item 47: Know and use the libraries

  4. Instead of the Hashtable use HashMap (with type parameters):

    Map<String, String> variables = new HashMap<String, String>();
    

    See: Differences between HashMap and Hashtable?

  5. The methods in the BasicCommands class have lots of smells:

    1. It's hard to follow because it uses exceptions for ordinary control flow. (Effective Java, 2nd Edition, Item 57: Use exceptions only for exceptional conditions)
    2. It contains lots of duplication. For example, num1 + " + " + num2 + " = " should be extracted out to a resultPrefix local variable.
    3. It uses floating point variables where you may need exact results. (Effective Java, 2nd Edition, Item 48: Avoid float and double if exact answers are required; Why not use Double or Float to represent currency?)

    Using BigDecimals would reduce the necessary code dramatically since it could store non-integer values, so it handles doubles and integers too, therefore you can omit a few conditions.

    public static String add(final String num1, final String num2) {
        final BigDecimal resolvedNum1 = resolveNumber(num1);
        final BigDecimal resolvedNum2 = resolveNumber(num2);
    
        return String.format("%s + %s = %s", num1, num2, resolvedNum1.add(resolvedNum2));
    }
    
    private static BigDecimal resolveNumber(final String number) {
        final String value = variables.get(number);
        if (value != null) {
            return new BigDecimal(value);
        }
        return new BigDecimal(number);
    }
    
\$\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.