How can I improve this Java program?

[Nathan4102]

Just the other day, I wrote this small cmd Calculator in java. Its nothing special, it runs off command line arguments, and doesn't loop or anything. Im wondering if you guys have any suggestions on how to "Improve" this program. I don't mean tell me what I can add to make it better, I mean am I using any poor java standards? Or are there more efficient/better ways to do this? Also, how would I make this using OOP?

Java:

public class Calculator {     public static void main (String args[]){                 if (args.length < 3) {                     System.out.println("Not enough arguments! Calculator(Operation, Num1, Num2)");                 } else if (args[0].equalsIgnoreCase("add")) {                         System.out.println("The answer is " +add(args[1], args[2]));                 } else if (args[0].equalsIgnoreCase("subtract")) {                         System.out.println("The answer is " +subtract(args[1], args[2]));                 } else if (args[0].equalsIgnoreCase("multiply")) {                         System.out.println("The answer is " +multiply(args[1], args[2]));                 } else if (args[0].equalsIgnoreCase("divide")) {                         System.out.println("The answer is " +divide(args[1], args[2]));                 } else {                         System.out.println("Please enter 'Add', 'Subtract', 'Multiply', or 'Divide' as your first argument.");                 }         }                 public static int add (String num1, String num2) {                 return Integer.parseInt(num1) + Integer.parseInt(num2);         }                 public static int subtract (String num1, String num2) {                 return Integer.parseInt(num1) - Integer.parseInt(num2);         }                 public static int multiply (String num1, String num2) {                 return Integer.parseInt(num1) * Integer.parseInt(num2);         }                 public static int divide (String num1, String num2) {                 return Integer.parseInt(num1) / Integer.parseInt(num2);         } }

Thanks a bunch guys
Nathan

Edit: Not sure why everything past my main method is indented extra. Just assume thats one indent less.

[DemonWasp]

What happens if you give more than 3 arguments?

Why not just get the integer values from args[1] and args[2] in a single place? Then you don't need methods, the code is more readable, and you have fewer calls to change if you want to change how integers are parsed (such as supporting hexadecimal or octal or binary, etc).

OOP:
This is overkill, but you could represent each operation by an object. Say you have an interface Operation that specifies public int compute ( int a, int b ) (the most general version would be public Number compute ( Number a, Number b ). Then you have class AddOperation implements Operation, etc.

You could then have a map from an operation name to its implementation:

Java:

Map<String,Operation> operations = new HashMap<>(); operations.put ( "add", new AddOperation () ); operations.put ( "subtract", new SubtractOperation() ); // ... // Then you can just look up operations by name: Operation operation = operations.get ( args[0].toLowerCase() ); if ( operation == null ) {     // error: no such operation } Number result = operation.compute ( a, b );

This is just one option, there are many others, each with their own advantages and disadvantages. This one only supports binary operations (two arguments) whereas there are functions that take many arguments, one argument, or no arguments.

[Nathan4102]

DemonWasp @ Mon Sep 02, 2013 3:07 pm wrote:

What happens if you give more than 3 arguments?

Why not just get the integer values from args[1] and args[2] in a single place? Then you don't need methods, the code is more readable, and you have fewer calls to change if you want to change how integers are parsed (such as supporting hexadecimal or octal or binary, etc).

OOP:
This is overkill, but you could represent each operation by an object. Say you have an interface Operation that specifies public int compute ( int a, int b ) (the most general version would be public Number compute ( Number a, Number b ). Then you have class AddOperation implements Operation, etc.

You could then have a map from an operation name to its implementation:

Java:

Map<String,Operation> operations = new HashMap<>(); operations.put ( "add", new AddOperation () ); operations.put ( "subtract", new SubtractOperation() ); // ... // Then you can just look up operations by name: Operation operation = operations.get ( args[0].toLowerCase() ); if ( operation == null ) {     // error: no such operation } Number result = operation.compute ( a, b );

This is just one option, there are many others, each with their own advantages and disadvantages. This one only supports binary operations (two arguments) whereas there are functions that take many arguments, one argument, or no arguments.

Good idea, I'll implement that now. And everything after "OOP:" flew right over my head. I still have much to learn :p