Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm working with lots of code that looks like this:

    int tmp;

    tmp = (statisticsObject.idag3_orig != 0) ? statisticsObject.idag3_orig : [self compareValue:statisticsObject.idag2 withValue:statisticsObject.idag3];
    statisticsView.idag3.text = [NSString stringWithFormat:@"%i", tmp];

    tmp = (statisticsObject.igar3_orig != 0) ? statisticsObject.igar3_orig : [self compareValue:statisticsObject.igar2 withValue:statisticsObject.igar3];
    statisticsView.igar3.text = [NSString stringWithFormat:@"%i", tmp];

    tmp = (statisticsObject.veckan3_orig != 0) ? statisticsObject.veckan3_orig : [self compareValue:statisticsObject.veckan2 withValue:statisticsObject.veckan3];
    statisticsView.veckan3.text = [NSString stringWithFormat:@"%i", tmp];

    tmp = (statisticsObject.manaden3_orig != 0) ? statisticsObject.manaden3_orig : [self compareValue:statisticsObject.manaden2 withValue:statisticsObject.manaden3];
    statisticsView.manaden3.text = [NSString stringWithFormat:@"%i", tmp];

    tmp = (statisticsObject.kvartalet3_orig != 0) ? statisticsObject.kvartalet3_orig : [self compareValue:statisticsObject.kvartalet2 withValue:statisticsObject.kvartalet3];
    statisticsView.kvartalet3.text = [NSString stringWithFormat:@"%i", tmp];

    tmp = (statisticsObject.aret3_orig != 0) ? statisticsObject.aret3_orig : [self compareValue:statisticsObject.aret2 withValue:statisticsObject.aret3];
    statisticsView.aret3.text = [NSString stringWithFormat:@"%i", tmp];

    tmp = (statisticsObject.r123_orig != 0) ? statisticsObject.r123_orig : [self compareValue:statisticsObject.r122 withValue:statisticsObject.r123];
    statisticsView.r123.text = [NSString stringWithFormat:@"%i", tmp];

    [self formatText:statisticsView.idag3];
    [self formatText:statisticsView.igar3];
    [self formatText:statisticsView.veckan3];
    [self formatText:statisticsView.manaden3];
    [self formatText:statisticsView.kvartalet3];
    [self formatText:statisticsView.aret3];
    [self formatText:statisticsView.r123];

I'm trying to remove the repetition as much as I can. Originally, the business logic was repeated once for each of the properties shown, but I've moved that into a method named compareValue:withValue:. I've also introduced the conditional operator to remove larger ifelse blocks. I imagine the next step is to reduce the seven chunks of code into one, and iterate over a list of keywords, but how?

share|improve this question

I can't really judge if it's a good idea, but one way could be creating an NSArray (or NSSet) containing NSDictionaries with the keypaths that should be compared, and then enumerating through that. Something like this:

#define keyOriginal = "original"
#define keyComparisonItem1 = "comparisonItem1"
#define keyComparisonItem2 = "comparisonItem2"

    NSMutableArray * evaluations = [NSMutableArray array];

    [evaluations addObject:@{keyOriginal : @"idag3_orig", keyComparisonItem1 : @"idag2", keyComparisonItem2 : @"idag3"}];
    [evaluations addObject:@{keyOriginal : @"igar3_orig", keyComparisonItem1 : @"igar2", keyComparisonItem2 : @"igar3"}];
        // and so on...

    for (NSDictionary * evaluation in evaluations) {

        if ([statisticsObject valueForKey:keyOriginal] != 0) {
            tmp = (int)[statisticsObject valueForKey:keyOriginal];
        } else {

            tmp = [self compareValue:[statisticsObject valueForKey:keyComparisonItem1]
                           withValue:[statisticsObject valueForKey:keyComparisonItem2]];

        [self formatText:[statisticsView valueForKey:keyOriginal]];

share|improve this answer

Your Answer


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.