Using a BOOL to replace verbose code

Question

I have a hideous line of code that's a gigantic PITA to type repeatedly and to read. I tried to replace it with a nice, neat BOOL property and corresponding method in my class, but it's "flakey" (and it doesn't crash). I'd like to replace this hideous monstrosity with something neater. Here's what I've done so far.

This is works...

    if (self.stretchSideMultiplierCount < [[[NSUserDefaults standardUserDefaults]valueForKey:@"stretchMultiplier"]integerValue] * self.currentStretch.sideMultiplier.integerValue) {
        // blah blah blah
    }

...but it looks hideous, so I tried to get rid of it by creating a BOOL property in the implementation:

@property (nonatomic, assign) BOOL lastRoundOfCurrentStretch;

...and a method down below...

- (BOOL) lastRoundOfCurrentStretch {
    if (self.currentStretch.sideMultiplier.intValue * [[[NSUserDefaults standardUserDefaults]valueForKey:@"defaultStretchRepetition"]intValue] == self.stretchSideMultiplierCount) {
        NSLog(@"** LAST ROUND **");
        return YES;
    } else {
        return NO;
    }
}

When I'm done, I'd like to be able to use this BOOL as an alternative to typing out the hideous line of code at the top of my query.

if (self.lastRoundOfCurrentStretch == NO) {
    // blah blah blah
}

After I made the change, the class isn't working the way it "used to", but it's not crashing. I'm certain this boils down to startlingly stupid user error on my part, but I'm coming up short finding an answer here. This is as close as I got to an answer that's applicable to what I'm trying to do.

Using a BOOL property

If you are removing things hard to read, first try to do only one operation on every line. Getting a value, a multiplication and a comparison = three operations. Use variables to split a long line into smaller operations. Also, never compare with `NO`, use logical operations (`if (!self.lastRoundOfCurrentStretch) {`). — Sulthan, Sep 29 '15 at 03:40

score 1 · Answer 1 · answered Sep 29 '15 at 03:26

1

in the top line of code you are using a < and in the updated one you are using a == i suspect this is causing undesired behaviour?

answered Sep 29 '15 at 03:26

Fonix

11,447
3
45
74

score 1 · Answer 2 · answered Sep 29 '15 at 03:27

1

You are using '==' instead of '<' in your property's getter, are you sure the code below is identical to code above?

answered Sep 29 '15 at 03:27

parachvte

56
3

score 1 · Accepted Answer · answered Sep 29 '15 at 03:49

1

Your original code can be rewritten as

NSInteger stretchMultiplier = [[[NSUserDefaults standardUserDefaults]valueForKey:@"stretchMultiplier"]integerValue];
NSInteger sideMultiplier = self.currentStretch.sideMultiplier.integerValue;
if (self.stretchSideMultiplierCount < stretchMultiplier * sideMultiplier) {
  // blah blah blah
}

Your property getter can be rewritten as:

- (BOOL) lastRoundOfCurrentStretch {
    NSInteger defaultStretchRepetition = [[[NSUserDefaults standardUserDefaults]valueForKey:@"defaultStretchRepetition"]integerValue];
    NSInteger sideMultiplier = self.currentStretch.sideMultiplier.integerValue;

    return (defaultStretchRepetition * sideMultiplier == self.stretchSideMultiplierCount);
}

Can you see the difference now? You are loading a different variable from user defaults. It's obvious once you make the code more readable.

answered Sep 29 '15 at 03:49

Sulthan

128,090
22
218
270

Thank you so much! This clarified things quite a bit for me. One detail question. Is this the correct way to declare the property in the implementation? `@property (nonatomic, assign) BOOL lastRoundOfCurrentStretch;` – Adrian Sep 29 '15 at 03:58
1

@AdrianB You can declare it in the class continuation, but there is no problem declaring it in the implementation or even not declaring it as a property at all. You don't really need a property, you need only the method. – Sulthan Sep 29 '15 at 04:01
For me, this is the most ****ing awesome answer I've ever gotten on StackOverflow. For you, however, it's probably the dumbest question you've ever answered. I'm going to refactor a bunch of stuff based on this. Thank you again. – Adrian Sep 29 '15 at 04:11

Using a BOOL to replace verbose code

3 Answers3