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

Showing an image based on the battery level

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

Problem

I'm working on a battery indicator feature in my app, and I was wondering if there is a better way than what I am doing to check what image should be displayed when. I am currently using if statements, but I was wondering if case statements would be a better option.

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {
    UIDevice *device = [UIDevice currentDevice];

    //Observer for the battery level
    //Updates the percentage label and the battery icon
    if (KEY_PATH_IF_THEN(@"batteryLevel")) {
        LabelBatteryStatus.text = [NSString stringWithFormat:@"%.0f%%", device.batteryLevel * 100];

        //shows battery image
        if (kBatteryLevelBetween(100, 100)) {
            batteryImage.image = [UIImage imageNamed:@"battery100fullycharged.png"];

        } else if (kBatteryLevelBetween(99, 81)) {
            batteryImage.image = [UIImage imageNamed:@"battery90.png"];

        } else if (kBatteryLevelBetween(80, 65)) {
            batteryImage.image = [UIImage imageNamed:@"battery75.png"];

        } else if (kBatteryLevelBetween(64, 41)) {
            batteryImage.image = [UIImage imageNamed:@"battery50.png"];

        } else if (kBatteryLevelBetween(40, 35)) {
            batteryImage.image = [UIImage imageNamed:@"battery30.png"];

        } else if (kBatteryLevelBetween(34, 15)) {
            batteryImage.image = [UIImage imageNamed:@"battery20.png"];

        } else if (kBatteryLevelBetween(14, 0)) {
            batteryImage.image = [UIImage imageNamed:@"battery10.png"];

        }
    }
}


kBatteryLevelBetween:

#define kBatteryLevelBetween(higherNum,lowerNum)    (device.batteryLevel *100 = lowerNum)

Solution

I think this code is easier to read and should result in significantly less comparisons, as it just compares in every step the lower bound, where as your define macro checks twice each. And note that you are checking also equality. this should result into

  • at least 2 checks at cpu level and



  • checking equality for floats in nver a good idea, as due to rounding errors they tend to be not equal, even if in pure math they should be (a 1 might be save as 0.99 or 1.001)



  • and if you would check integer numbers for being in some range it make sense, to have an equal check just on one bound like if (a lowerNum)



my code suggestion

-(NSUInteger)batteryLevel
{
    UIDevice *device = [UIDevice currentDevice];
    float level = device.batteryLevel;
    if(level > .99) return 100;
    if(level > .80) return 90;
    if(level > .64) return 75;
    if(level > .40) return 50;
    if(level > .34) return 30;
    if(level > .14) return 20;
    return 10;
}

-(UIImage *)imageForBatteryLevel
{
    NSUInteger level = [self batteryLevel];
    NSString *imageName;
    if(level > 99){
        imageName = @"battery100fullycharged.png";
    } else {
        imageName = [NSString stringWithFormat:@"battery%d.png", level];
    }
    return [UIImage imageNamed:imageName];
}

- (void)observeValueForKeyPath:(NSString *)keyPath 
                      ofObject:(id)object 
                        change:(NSDictionary *)change 
                       context:(void *)context 
{
    if (KEY_PATH_IF_THEN(@"batteryLevel")) {
        LabelBatteryStatus.text = [NSString stringWithFormat:@"%.0f%%", device.batteryLevel * 100];
        batteryImage.image = [self imageForBatteryLevel];
    }
}


I was taught at university, that in assembler code checking equality is actually done by checking, that the value is neither smaller nor greater, this makes it 2 comparisons. I am not sure, that this is true for every CPU, but let's walk through an example with that.

Let's assume, the level is down to 2%. with your code, it will go liek this:

  • is .02 * 100 bigger or equal than 100 and



  • is .02 * 100 smaller or equal than 100 -> No



  • is .02 * 100 smaller or equal than 99 and



  • is .02 * 100 bigger or equal than 81 -> No



  • is .02 * 100 smaller or equal than 80 and



  • is .02 * 100 bigger or equal than 65 -> No



  • is .02 * 100 smaller or equal than 64 and



  • is .02 * 100 bigger or equal than 41 -> No



  • is .02 * 100 smaller or equal than 40 and



  • is .02 * 100 bigger or equal than 35 -> No



  • is .02 * 100 smaller or equal than 34 and



  • is .02 * 100 bigger or equal than 15 -> No



  • is .02 * 100 smaller or equal than 14 -> Yes



As you see you have 13 comparisons for smaller or bigger and 13 for equal. that yields in 39 comparisons. Additionally you have 13 times the same floating number multiplication (.02 * 100). And a gap between the boundaries: If the percentage would result in 14.5, no rule would be triggered.

Let's use my rules

  • is .02 > .99 -> No



  • is .02 > .80 -> No



  • is .02 > .64 -> No



  • is .02 > .40 -> No



  • is .02 > .34 -> No



  • is .02 > .14 -> No



=> return 10

As you see, I have maximum 6 comparisons and no floating point multiplication.

Code Snippets

-(NSUInteger)batteryLevel
{
    UIDevice *device = [UIDevice currentDevice];
    float level = device.batteryLevel;
    if(level > .99) return 100;
    if(level > .80) return 90;
    if(level > .64) return 75;
    if(level > .40) return 50;
    if(level > .34) return 30;
    if(level > .14) return 20;
    return 10;
}


-(UIImage *)imageForBatteryLevel
{
    NSUInteger level = [self batteryLevel];
    NSString *imageName;
    if(level > 99){
        imageName = @"battery100fullycharged.png";
    } else {
        imageName = [NSString stringWithFormat:@"battery%d.png", level];
    }
    return [UIImage imageNamed:imageName];
}

- (void)observeValueForKeyPath:(NSString *)keyPath 
                      ofObject:(id)object 
                        change:(NSDictionary *)change 
                       context:(void *)context 
{
    if (KEY_PATH_IF_THEN(@"batteryLevel")) {
        LabelBatteryStatus.text = [NSString stringWithFormat:@"%.0f%%", device.batteryLevel * 100];
        batteryImage.image = [self imageForBatteryLevel];
    }
}

Context

StackExchange Code Review Q#14911, answer score: 7

Revisions (0)

No revisions yet.