Refactoring Objective C... I know this is sloppy, how can I make it pretty?

Question

I just wrote this:

- (IBAction)weatherButtonPressed:(id)sender {
    BOOL cloudy = NO;
    int rando = arc4random()%101;

    if (rando <= 50) {
        cloudy = YES;}
    else {
        cloudy = NO;
    }

    if (cloudy) {
        self.weatherLabel.text = @"no, it's cloudy.";
    } else {
        self.weatherLabel.text = @"yes, it's clear!";
    }

    NSLog(@"rando: %d", rando);
}

I know it's super sloppy. Also, I couldn't figure out how to get my random number to actually appear in my weatherLabel text next to the result of cloudy or clear. It seems like there should be a way to set the Boolean to just be random without the rando variable.

http://stackoverflow.com/questions/9573395/ios-random-number-generator-to-a-new-view — Danny Xu, Dec 27 '13 at 03:11
@JonReid true, but the short answer was also cool. Which one would you have chosen and why? — pixelfairy, Jan 02 '14 at 02:49
The short answer _is_ cool. I'd expand it by extracting an explaining variable for `arc4random()%2`. But going further for unit testing, I'd want a way to inject the randomizer, so that tests can inject predetermined values. — Jon Reid, Jan 02 '14 at 03:47

score 1 · Answer 1 · answered Dec 27 '13 at 03:09

1

Does it look better?

- (IBAction)weatherButtonPressed:(id)sender {
    BOOL cloudy = arc4random()%2 == 0 ;
    if (cloudy) {
        self.weatherLabel.text = @"no, it's cloudy.";
    } else {
        self.weatherLabel.text = @"yes, it's clear!";
    }
}

answered Dec 27 '13 at 03:09

KudoCC

6,912
1
24
53

Thank you!! This is definitely nice and short. – pixelfairy Dec 27 '13 at 03:24

score 1 · Accepted Answer · answered Dec 27 '13 at 03:14

1

The method choose random object from an array.

- (IBAction)weatherButtonPressed:(id)sender {
  self.weatherLabel.text = @[@"no, it's cloudy.",@"yes, it's clear!"][arc4random()%2];
}

answered Dec 27 '13 at 03:14

icodesign

876
1
6
12

works! that's clever. So it picks a random value from the array just because? or is the random number telling it which piece of the array to grab? – pixelfairy Dec 27 '13 at 03:26
@[@"no, it's cloudy.",@"yes, it's clear!"] is an array. arc4random()%2 generate a random integer between 0 and 1. @[][index] syntax means it chooses an object from the array at index. – icodesign Dec 27 '13 at 03:27
I think I'm starting to get it .. I could also do a third state for the weather and increase the random number, yes? – pixelfairy Dec 27 '13 at 03:28
- (IBAction)weatherButtonPressed:(id)sender { self.weatherLabel.text = @[@"no, it's cloudy.",@"yes, it's clear!",@"hard to tell, partly cloudy."][arc4random()%3]; ; } – pixelfairy Dec 27 '13 at 03:29
@pixelfairy That's pretty good. When the array goes larger, it's better to code like this: `NSArray *array = @[XX, XX, XX, XX]; self.weatherLabel.text = array[arc4random()%array.count];` – icodesign Dec 27 '13 at 03:32
sweet! I will try it in that format. I feel so smart now. Thanks!! – pixelfairy Dec 27 '13 at 04:32

zaph · Answer 3 · 2013-12-27T03:21:37.520

My take on a refactoring:

- (IBAction)weatherButtonPressed:(id)sender {
    NSString *weatherText;

    if ([self isCloudy]) {
        weatherText = @"no, it's cloudy.";
    }
    else {
        weatherText = @"yes, it's clear!";
    }

    self.weatherLabel.text = weatherText;
}

- (BOOL)isCloudy {
    return arc4random_uniform(1) == 0;
}

Or a more direct refactoring of isCloudy:

- (BOOL)isCloudy {
    BOOL cloudy = NO;

    int rando = arc4random_uniform(101);
    NSLog(@"rando: %d", rando);

    if (rando <= 50) {
        cloudy = YES;
    }

    return cloudy;
}

Refactoring Objective C... I know this is sloppy, how can I make it pretty?

3 Answers3