5

Potentially dumb: Assuming I have a string containing an operator what's the best way to apply this operator ?

What i tend to do is :

if(n.getString(1).equals("<<")) {
  result = tmp1 << tmp2;
}

for each kind of operator I have. Is there a better way ?

LB40
  • 12,041
  • 17
  • 72
  • 107

10 Answers10

15

Not sure whether you'd call this elegant, but here is one way:

interface Operation {
  int apply(int a, int b);
}

Map<String, Operation> operations = new HashMap<String, Operation>() {{
  put("+", new Operation() { public int apply(int a, int b) { return a + b; }});
  put("-", new Operation() { public int apply(int a, int b) { return a - b; }});
  put("*", new Operation() { public int apply(int a, int b) { return a * b; }});
  put("<<", new Operation() { public int apply(int a, int b) { return a << b; }});
  // some more operations here
}};

Then you could replace your if statement with:

result = operations.get(n.getString(1)).apply(tmp1, tmp2);
missingfaktor
  • 90,905
  • 62
  • 285
  • 365
  • that's how you implement in Java my pseudocode. And the verboseness is what kills it. BTW, man, that kind of Map initialization can be troublesome. It's trivial to write a helper class that let's you write Map.build().put("a",1).put("b",2).map and obtain a Map... – alex Jun 02 '10 at 19:30
  • @alex, Double brace initialization isn't as expensive as people tend to think. See this thread: http://stackoverflow.com/questions/924285/efficiency-of-java-double-brace-initialization – missingfaktor Jun 02 '10 at 19:31
  • Wait a minute Rahul. :-P. you'll have it – LB40 Jun 02 '10 at 19:37
  • An aside note: As @alex noted, this would look much more elegant in a language that supports concise map declaration syntax and closures. For instance, here's the Scala equivalent of the Java code above: http://paste.pocoo.org/show/221167/ – missingfaktor Jun 02 '10 at 19:57
  • I guess Java 7 would be the new Scala :P It would have both, Map declaration and closures. – OscarRyz Jun 02 '10 at 20:44
  • It would be elegant in C# with lamdas - the Java anonymous classes is horrid. – Callum Rogers Jun 02 '10 at 20:54
  • I don't care if it's expensive. It's confusing. The object not being an instance of HashMap, but a subclass will probably surprise people and might even cause trouble somewhere. Can you assume it won't cause trouble? What advantage does this map construction technique have? – alex Jun 03 '10 at 20:12
7

Could do something like:

enum Operator {

 BITSHIFT { ... }, ADD { ... }, XOR { ... }, //...etc

 Operator public static which(String s) { //...return the correct one
 }

 public abstract int apply(int a, int b); //...defined explicitly for each enum

}

the return the right one will be really nice once switch statements are go for Strings.

This solution looks like the following in use (sans Operator. if you use a static import):

 int result = Operator.which(s).apply(a,b);

but I'd go with someone else's widely-tested and used parser.

Carl
  • 7,538
  • 1
  • 40
  • 64
5

The object oriented way to do it would be to use an enumeration of the possible operations. That way each operation could only consume one object in memory.

public enum Operation {


  ADD() {
    public int perform(int a, int b) {
      return a + b;
    }
  },
  SUBTRACT() {
    public int perform(int a, int b) {
      return a - b;
    }
  },
  MULTIPLY() {
    public int perform(int a, int b) {
      return a * b;
    }
  },
  DIVIDE() {
    public int perform(int a, int b) {
      return a / b;
    }
  };

  public abstract int perform(int a, int b);

}

To call such code, you would then do something like:

int result = Operation.ADD(5, 6);

Then you could create a map of Strings to Operations, like so:

Map<String, Operation> symbols = new Map<String, Operation>();
symbols.put("+", Operation.ADD);
symbols.put("-", Operation.SUBTRACT);
symbols.put("/", Operation.DIVIDE);
symbols.put("*", Operation.MULTIPLY);
...

Finally, to use such a system:

symbols.get(n.getString(1).apply(tmp1, tmp2));

One advantage to using enumerations in this manner is that you have the luxury of comparing the operations on the data, should you choose to do so

Operation operation = symbols.get("*");
if (operation != Operation.MULTIPLY) {
  System.out.println("Foobar as usual, * is not multiply!");
}

In addition, you get a centralized location for all operations, the only downside to this is that the Operation.java file might grow large with a sufficiently large set of operators.

The only issues that might exist long-term is that while such a system is useful and easy to read and understand, it really doesn't take into account precedence. Assuming your formula are all evaluated in the order of precedence, such a problem doesn't matter. Examples of expressing the formula in the order of precedence can be found in Reverse Polish Notation, Polish Notation, etc.

Where precedence does matter is when you are allowed to express items like:

4 + 5 * 2

where according to typical convention, the 5 * 2 should be evaluated before the 4 + 5. The only correct way to handle precedence is to form an evaluation tree in memory, or to guarantee that all input handles precedence in a simple, unambiguous manner (Polish Notation, Reverse Polish Notation, etc).

I'm assuming you know about the precedence issues, but thank you for letting me mention it for the benefit of those who haven't had to write such code yet.

Edwin Buck
  • 69,361
  • 7
  • 100
  • 138
  • 1
    +1, Here's a variation of the above wherein you don't have to add entries to the map manually: http://paste.pocoo.org/show/221499/ – missingfaktor Jun 03 '10 at 16:41
  • @Rahul I like the way you mixed the symbol with the Operation, but after mixing the two the name "Operator" seems a little bit inappropriate. Since the Class now contains the symbol and the operation, I would call it an Evaulator. But, "What is in a name? That which we call a rose by any other name would smell as sweet." -- Shakespeare – Edwin Buck Jun 03 '10 at 16:57
3

Using an actual parser would probably be more robust, although it may be overkill depending on how complex your problem is. JavaCC and ANTLR are probably the two most popular parser generators in the Java world.

If a parser generator is overkill (possibly because you don't have/want to create a formal grammar), you could probably use Ragel to implement a custom parser.

Hank Gay
  • 70,339
  • 36
  • 160
  • 222
  • that's the same kind of thing when you're using a parser. You can have all the operators (with 2 operands) scanned as OP and then you need to implement the actual operation – LB40 Jun 02 '10 at 19:33
2

Unless you have a large set of operators or want to be able to allow for more complex expressions in the future, nope.

One way to do this (pseudocode, if Java had this syntax, this would be a good option even with a small set of operators):

final Map<String,Function<(X,X), Y>> OPERATORS = { 
    "<<" : (x,y)->{x << y}, 
    "+" : (x,y)->{x + y},
    [...]
};

[...]

result1 = OPERATORS.get(n.getString(1))(tmp1, tmp2);

you can write this in Java, but due to lack of concise Map literals and anonymous classes declarations, it's much more verbose.

The other option is, has posted by Hank Gay, use a "real" parser/evaluator. You could write your own, or use something like commons-jexl. Unless you want to allow for more complex expressions (and then you'll be forced to go this route), this is seriously overkill.

alex
  • 5,213
  • 1
  • 24
  • 33
2

An enum would be clean for this type of scenario.

public enum Operator
{
    ADD("+")
    {
        public int apply(int a, int b)
        {
            return a + b;
        }
    }

    SHIFT_LEFT("<<")
    {
        public int apply(int a, int b)
        {
            return a << b;
        }
    }

    private String opString;

    private Operator(String op)
    {
        opString = op
    }

    static public Operator getOperator(String opRep)
    {
        for (Operator o:values())
        {
            if (o.opString.equals(opRep))
                return o;
        }
        throw new IllegalArgumentException("Operation [" + opRep + "] is not valid");
    }

    abstract public int apply(int a, int b);
}

To call

result = Operator.getOperator("<<").apply(456,4);

Another option would be to include a static method which takes the string representation of the operator as well as the operands and have a single method call, although I prefer them separate.

Robin
  • 24,062
  • 5
  • 49
  • 58
  • You can replace `EnumSet.allOf(Operator.class)` with `values()`. – missingfaktor Jun 02 '10 at 20:03
  • 1
    @Willi - I agree, but this was just meant to show the idea. I would probably go with an IllegalArgumentException in reality. I'll edit anyway. – Robin Jun 03 '10 at 14:33
1

You could use an JSR-223 interpretive language such as BeanShell, have it perform the operation, and then get the result back.

Chris Dennett
  • 22,412
  • 8
  • 58
  • 84
1

You can also use the hashCode of the operator and put it on a switch although this is not very "elegant" from the OOP perspective.

This may work if you're not adding operators too often ( which I don't think you would )

So this should be enough:

String op = "+";

switch( op.hashCode() ){
    case ADD: r = a +  b;break;
    case SUB: r = a -  b;break;
    case TMS: r = a *  b;break;
    case DIV: r = a /  b;break;
    case MOD: r = a %  b;break;
    case SLF: r = a << b;break;
    case SRG: r = a >> b;break;
    case AND: r = a &  b;break;
    case OR:  r = a |  b;break;
    case XOR:  r = a ^ b;break;
    default: out.print("Eerr!!!"); break;
}

And have AND defined as:

private static final int ADD = 0x2b;

Here's the complete running sample code:

import static java.lang.System.out;
class Evaluator {

    private static final int ADD = 0x2b;    // +
    private static final int SUB = 0x2d;    // -
    private static final int TMS = 0x2a;    // *
    private static final int DIV = 0x2f;    // /
    private static final int MOD = 0x25;    // %
    private static final int SLF = 0x780;    // <<
    private static final int SRG = 0x7c0;    // >>
    private static final int AND = 0x26;    // &
    private static final int OR  = 0x7c;    // |
    private static final int XOR  = 0x5e;    // ^

    private int r;
    private int a;
    private String op;
    private int b;

    private Evaluator( int a, String op, int b ) {
        this.a = a; this.op = op; this.b = b;
    }
    private Evaluator eval() {
        switch( op.hashCode() ){
            case ADD: r = a +  b;break;
            case SUB: r = a -  b;break;
            case TMS: r = a *  b;break;
            case DIV: r = a /  b;break;
            case MOD: r = a %  b;break;
            case SLF: r = a << b;break;
            case SRG: r = a >> b;break;
            case AND: r = a &  b;break;
            case OR:  r = a |  b;break;
            case XOR:  r = a ^ b;break;
            default: out.print("Eerr!!!"); break;
        }
        return this;
    }

    // For testing:
    public static int evaluate( int a, String op ,  int b ) {
        return new Evaluator(a, op, b).eval().r;
    }

    public static void main( String [] args ) {
        out.printf( " 1 + 2   = %d%n", evaluate( 1 ,"+" , 2 ));
        out.printf( " 1 - 2   = %d%n", evaluate( 1 ,"-" , 2 ));
        out.printf( " 1 * 2   = %d%n", evaluate( 1 ,"*" , 2 ));
        out.printf( " 1 / 2   = %d%n", evaluate( 1 ,"/" , 2 ));
        out.printf( " 1 %% 2  = %d%n", evaluate( 1 ,"%" , 2 ));
        out.printf( " 1 << 2  = %d%n", evaluate( 1 ,"<<" , 2 ));
        out.printf( " 1 >> 2  = %d%n", evaluate( 1 ,">>" , 2 ));
        out.printf( " 1 & 2   = %d%n", evaluate( 1 ,"&" , 2 ));
        out.printf( " 1 |  2  = %d%n", evaluate( 1 ,"|" , 2 ));
        out.printf( " 1 ^ 2   = %d%n", evaluate( 1 ,"^" , 2 ));
    }
}

And that's it. Runs very very fast ( I'm pretty sure a enum in in order )

From an OOP perspective I think Rahul answer would do, but if you want to get any serious with this you should use a parsers as Hank Gay suggest.

p.s. Getting the hash code of a string is easy:

System.out.println( "+".hashCode() );

I actually used this:

public class HexHashCode {
    public static void main( String [] args ) {
        for( String s: args ) {
            System.out.printf("private final int %s = 0x%x;    // %s\n",s,s.hashCode(), s );
        }
    }
}

And run it with:

java HexHashCode + - "*" / "<<" ">>" "%" "<" ">" "==" "!=" "&" "|"  "^" 
OscarRyz
  • 196,001
  • 113
  • 385
  • 569
  • Could be made faster by avoiding the unnecessary object allocations: http://paste.pocoo.org/show/221566/ – missingfaktor Jun 03 '10 at 18:52
  • @Rahul I agree, but the difference is negligible, really and the advantage of using objects is they can use stored in a data structure ( say, a stack ) for later evaluation. Still pretty fast. – OscarRyz Jun 03 '10 at 19:34
  • I don't mind the downvote, I just would like to know the reason. – OscarRyz Jun 03 '10 at 20:23
  • Dunno whether it was addressed to me but I upvoted the answer, not downvote it. – missingfaktor Jun 04 '10 at 07:24
1

its very easy to do in c sharp.

var ops = new Dictionary<string, Func<int, int, int>> {
  {"+", (a, b) => a + b},
  {"-", (a, b) => a - b},
  {"*", (a, b) => a * b},
  {"/", (a, b) => a / b}
};

test:

var c = ops["+"](2, 3);
Console.WriteLine(c);

prints 5.

0

@Robin: in Java you can't have abstract methods in non-abstract classes, and enum types cannot be abstract (because all enum's inherit from Enum, which means one enum type cannot inherit from another by Java's inheritance rules)

Also, what you can do is add a static method called register:

private static final Map<String, Operator> registered=new HashMap<String,Operator>();
private static void register(String op, Operator impl) {
   registered.put(op, impl);
}

And then call that method in the constructor; and use map lookup in the getOperator() method.

user268396
  • 11,576
  • 2
  • 31
  • 26
  • His code is absolutely correct. Did you try compiling it before posting this? – missingfaktor Jun 03 '10 at 17:41
  • I see now. Sorry for that. Still, my suggestion of using a Map -as shown above- still stands; especially if the number of operation defined is large-ish or you expect to run a lot of these comparisons. – user268396 Jun 03 '10 at 21:57