Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

this is my first post here!
How can i optimize the if condition on this snippet?

The only one difference is && [self isCurrentPosition:i].
How can made it on a single if, including the condition val?

- (void) cleanTextfieldExcluding:(int)current checkPosition:(BOOL)val {
  for ( int i=0; i<[self count]; i++ ) {

    // -----------
    if ( val ) {  // this IF block is very bad
      if ( i != current && [self isCurrentPosition:i] )
        [self replaceObjectAtIndex:i withObject:@""];
    } else {
      if ( i != current ) 
        [self replaceObjectAtIndex:i withObject:@""];
    }
    // -----------

  }
}

Note: self is a category of NSArray.

thanks.

share|improve this question
    
you cannot merge it go a single if. –  zdd Nov 28 '12 at 10:18

2 Answers 2

up vote 3 down vote accepted

It seems that the code in if will be executed only if i != current and val && [self isCurrentPosition:i] or !val, so you could merge all the ifs into one:

- (void) cleanTextfieldExcluding:(int)current checkPosition:(BOOL)val {
  for ( int i=0; i<[self count]; i++ ) {

    // -----------
    if ( (i != current) && (!val || [self isCurrentPosition:i]) ) {
        [self replaceObjectAtIndex:i withObject:@""];
    }
    // -----------

  }
}
share|improve this answer
    
+1. perfect way! –  elpsk Nov 28 '12 at 11:49
    
just saw the answer below me and OMG it was better than mines(performs one less logic operation) so i updated mines too. :P –  namehere Nov 28 '12 at 15:32
    
-1. Your answers are now the same because you copied his code (and didn't upvote his answer). –  seand Nov 28 '12 at 16:29
    
oops sorry. i forgot to upvote it. just did it. –  namehere Nov 28 '12 at 16:30

the (val · ¬cur · pos) | (¬val · ¬cur) = ¬cur (val·pos | ¬val) = ¬cur (pos | ¬val). So, the condition is

if ( (i != current) && (!val || [self isCurrentPosition:i]) ) {
        [self replaceObjectAtIndex:i withObject:@""];
    }

You just must practise factoring out the common subexpressions and subprograms and learn how to use the Carnot maps for boolean minimization.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.