HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Updating images in a weather app

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
imagesupdatingappweather

Problem

Right now in my weather app, I have one section of code in my View Controllers to set up the condition image and the background image. It has about 400 lines of if-else statement at the end.

The app's performance is fine, but is it bad to have this? Would it be something that, say, Apple would consider rejecting? The code is very easy to read, and makes perfect sense in my opinion.

- (void)updateImages:(ICB_WeatherConditions *)weather {
    if ([condition isEqualToString:@"113"]) {
        conditionsImageView.image = [UIImage imageNamed:@"Sun.png"];
        BGView.image = [UIImage imageNamed:@"Sun.jpg"];
    } else {
        if ([condition isEqualToString:@"116"]) {
            conditionsImageView.image = [UIImage imageNamed:@"Mostly_Sunny.png"];
            BGView.image = [UIImage imageNamed:@"Partly_Cloudy.jpg"];

        } else {
            if ([condition isEqualToString:@"119"]) {
                conditionsImageView.image = [UIImage imageNamed:@"Overcast.png"];
                BGView.image = [UIImage imageNamed:@"Overcast.jpg"];
            }
            else {
                if ([condition isEqualToString:@"122"]) {
                    conditionsImageView.image = [UIImage imageNamed:@"Overcast.png"];
                    BGView.image = [UIImage imageNamed:@"Overcast.jpg"];
                }
                else {
                    if ([condition isEqualToString:@"143"]) {
                        conditionsImageView.image = [UIImage imageNamed:@"Mist.png"];
                        BGView.image = [UIImage imageNamed:@"Foggy.jpg"];
                    }
                    else {
                        if ([condition isEqualToString:@"176"]) {
                            conditionsImageView.image = [UIImage imageNamed:@"Scattered_Thunderstorms.png"];
                            BGView.image = [UIImage imageNamed:@"Scat_Tstorms.jpg"];
                    }


It keeps going on after that. As you can see, I have a lot of nearly identical statements,

Solution

In this case, your indentation makes the code drift over to the right so does make the code difficult to read so does need some changes.

First I would note that a lot of code is repeated and also as noted a switch after converting the condition to an int is possible.

However in this case your code is effectively doing multiple lookups so I would look at using NSDictionaries (or NSArrays if the conditions are 0 to a number) and then do a straight lookup

e.g.
setup

NSDictionary* conditionsImages = [NSDictionary dictionaryWithObjectsAndKeys: 
                @"113", @"Sun.png",
                @"116", @"Mostly_Sunny.png",
     ...
                nil];


Or read the dictionaries from a file

Access them by

- (void)updateImages:(ICB_WeatherConditions *)condition {
    conditionsImageView.image = [UIImage imageNamed:
                   [conditionsImages objectForKey:condition]];
    BGView.image = [UIImage imageNamed:[otherImages objectForKey:condition]];

  ...
}

Code Snippets

NSDictionary* conditionsImages = [NSDictionary dictionaryWithObjectsAndKeys: 
                @"113", @"Sun.png",
                @"116", @"Mostly_Sunny.png",
     ...
                nil];
- (void)updateImages:(ICB_WeatherConditions *)condition {
    conditionsImageView.image = [UIImage imageNamed:
                   [conditionsImages objectForKey:condition]];
    BGView.image = [UIImage imageNamed:[otherImages objectForKey:condition]];

  ...
}

Context

StackExchange Code Review Q#13577, answer score: 17

Revisions (0)

No revisions yet.