Obj-C: if statements and NSString comparison functions for efficiency and readability

Question

What would be the best way to code this efficiently cause this is going to get called recursively, and creating alot of objects is no good. +*-/, sin,cos,sqrt,pi and x,y,a,b should be put together in three separate if statements unlike now where everything is seperated. Also for readability making a if statments that contains 4 isEqualToString is not very good.

    NSString *operation = topOfStack;
    if ([operation isEqualToString:@"+"]) {

    } else if ([@"*" isEqualToString:operation]) {

    } else if ([operation isEqualToString:@"-"]) {

    } else if ([operation isEqualToString:@"/"]) {


    } else if ([operation isEqualToString:@"sin"]) {

    } else if ([operation isEqualToString:@"cos"]) {

    } else if ([operation isEqualToString:@"sqrt"]) {

    } else if ([operation isEqualToString:@"pi"]) {


    } else if ([operation isEqualToString:@"x"]) {

    } else if ([operation isEqualToString:@"y"]) {

    } else if ([operation isEqualToString:@"a"]) {

    } else if ([operation isEqualToString:@"b"]) {

    }

There's no good way to do this in objective-c, honestly. The ideal way would be to use a switch statement, but unfortunately objects aren't supported in switch statements in objc. — Richard J. Ross III, Jan 30 '13 at 18:03
Re: "creating alot of objects is no good" -- none of this code creates any objects. — Kurt Revis, Jan 30 '13 at 18:04
Speaking of switch statements on NSStrings, why don't you check out this header library I made for doing switches on objects: http://stackoverflow.com/questions/4224495/using-an-nsstring-in-a-switch-statement/13114988#13114988 — Richard J. Ross III, Jan 30 '13 at 18:04

Eric · Answer 1 · 2013-01-31T04:03:48.813

I don't believe there's a best way to do this, but here's another option to supplement the answers already given.

Create an NSDictionary that maps your operation name (sin, sqrt, -) to a selector, like so:

NSDictionary *operations = @{
    @"sin": [NSValue valueWithPointer:@selector(operationSin:)],
    /* other operations here */
};

Then your switch statement becomes a lookup.

NSString *operation = topOfStack;
if(operations[operation]) {
    SEL op = [operations[operation] pointerValue];
    [self performSelector:op withObject:value];  /* Or some approximation thereof */
}
else {
    /* Default action for unknown operation */
}

Alternatively, you can create an enum on your recognized operations, using your NSDictionary to map operation name to an NSNumber boxing your enum value.

typedef NS_ENUM(NSUInteger, OperationType) {
    OperationTypeSin = 0L,
    OperationTypeSqrt,
    /* and so on */
    OperationTypeUnknown
};

/* And later...  */
NSDictionary *operations = @{
    @"sin": @(OperationTypeSin),
    /* you get the idea */
};

/* Finally ... */
NSString *operation = topOfStack;
OperationType opType = [operations[operation] unsignedIntegerValue];
switch(opType) {
    case OperationTypeSin:
        /* Much cleaner, and type-safe too */
}

I think you're thinking of `[self performSelector:op withObject:value]` — Tommy, Jan 30 '13 at 23:42

score 2 · Answer 2 · answered Jan 30 '13 at 18:10

2

Preparation:
First convert the NSString operation into a int hashcode, store that hashcode in a #define or constant.

Code:

1) convert the NSString operation into a int hashcode (tokenId).
2) Then do a switch statement on that tokenId.

int token = operationToToken(operation);
switch (token) {
case: OP_MINUS: break;
case: OP_SIN: break;
case: OP_COS: break;

}

answered Jan 30 '13 at 18:10

AlexWien

28,470
6
53
83

Hashes can (and do) collide, and are not really intended for something like this. The other thing you don't take into consideration is case-sensitivity, which, if there is user-input should be taken care of. Not a bad solution, and there really aren't many 'good' answers in C, but I'm sure that something could be made somewhat better. – Richard J. Ross III Jan 30 '13 at 18:18
You can check if they collide or, not because you have a limited alphabet of lets say 20 operation strings. if they collide, then create a beter hashfunction, e.g by asumming each letter is a digit 0-26 in number of base 26. Checking of correct input (toLowercase() can be done in advance, the question was for efficiency in a (huge) number of recursive calls. – AlexWien Jan 30 '13 at 18:21
This is good in theory, but you've changed the problem from "compare strings to perform an action" to "write a sufficient hash function" which is more likely to create maintenance issues down the road if enough operations are added. – Eric Jan 30 '13 at 18:29
If you need the performance, then this is one way to go, If you don' need it, it stay with string compare and if else. However, of course there should be a unit test that checks your whole alphabet for returning a unique hashcode. – AlexWien Jan 30 '13 at 18:31

score 1 · Answer 3 · answered Jan 30 '13 at 18:07

1

Firstly, maybe fall back to C strings, and then exploit the fact that sin, cos, sqrt, pi, a, b, x and y (almost) all start by different characters:

const char *s = [operation UTF8String];
switch (s[0]) {
case '+':
    // addition
    break;
case '-':
    // subtraction
    break;
case 's':
    // sine or sqrt
    switch (s[1]) {
    case 'q':
        // sqrt
        break;
    case 'i':
        // sine
        break;
    }
    break;
case 'c':
    // cosine
    break;
// et cetera...
default:
    // not found
}

answered Jan 30 '13 at 18:07

I wouldn't recommend this, what do you do when someone tries to do an arc-cos or arc-sine? – Richard J. Ross III Jan 30 '13 at 18:08
1

@RichardJ.RossIII I know, I know... I just noticed that we have `sqrt` and `sin`. – Jan 30 '13 at 18:09
it would work - but as the poster above me says - if I want to expand the calculator with more operations I´d potentially have to rewrite the whole method – Tom Lilletveit Jan 30 '13 at 18:09
@TomLilletveit You can still `strcmp()`, any decent compiler can inline it for short strings. – Jan 30 '13 at 18:10
1

@H2CO3 I understand what you are trying to do with the update, but honestly it's less readable than the OP's version. I shouldn't have to dig through nested switch statements to figure out what's going on. – Richard J. Ross III Jan 30 '13 at 18:11
@RichardJ.RossIII And honestly, is your object-switch with 2-3 levels of method calls and the lookup overhead of `NSDictionary` faster than comparing two integers? – Jan 30 '13 at 18:12
My version wasn't meant for speed, but for readability, and thus why I suggested it. Optimizing a string comparison using nested switch statements isn't worth it unless you are performing this millions of times a minute, which, if the OP is, then he'd better look into a true math parser, or maybe write it in assembly! – Richard J. Ross III Jan 30 '13 at 18:17

score 1 · Answer 4 · answered Jan 30 '13 at 23:34

If you're using Objective-C then you could possibly do this with some cleverly named selectors.

First, create an object to encapsulate an Operation.

Then, create an -initWithOperation:(NSString *)op method. Inside this method, convert any symbol operations (*, +, etc) to unique string-identifiers (letters only).

Then, you could call NSSelectorFromString(stringOp) although you will have to append the : characters yourself if you want the methods to have arguments.

Once you have the selector, you can perform the selector using -performSelector: , +performSelector:, or call the selector manually.

Provided that you name each selector with the same name as the operation, you don't need any if or switch statements, but you will have to handle the case where you send an invalid selector.

E.g.:

@interface Operation : NSObject

- (id)initWithOperation:(NSString *)op;
- (void)performOperationWithValue:(float)value;
+ (float)sin:(float)value;
// ... Other operations
@property (nonatomic, copy) NSString *theOperation;

@end

@implementation Operation
- (id)initWithOperation(NSString *)op {
    self = [super init];
    if (self) {
        // Convert symbols to unique strings
        theOperation = [[NSString alloc] initWithString:op];
    }
    return self;
}

- (void)performOperationWithValue:(float)value {
    NSString *withOneParam = [self.theOperation stringByAppendingString:@":"];
    SEL sel = NSSelectorFromString(withOneParam);
    [Operation performSelector:withOneParam withObject:[NSNumber numberWithFloat:value]];
}

// Class methods are the actual operation implementation...

In the above example, I assume you have passed @"sin" to the Operation object. This is only possible in Objective-C though as you can take advantage of named selectors.

score 0 · Answer 5 · answered Jan 30 '13 at 18:06

0

You can assign each symbol a number and can implement a switch case.

answered Jan 30 '13 at 18:06

Saurabh Shukla

1,368
3
14
26

While yes you can do this, it makes things very ugly in the long run, and at that point, I think that using a simulated switch on objects will end up with more readable code in the long run, like this one: http://stackoverflow.com/questions/4224495/using-an-nsstring-in-a-switch-statement/13114988#13114988 – Richard J. Ross III Jan 30 '13 at 18:08

Obj-C: if statements and NSString comparison functions for efficiency and readability

5 Answers5