FizzBuzz in Objective-C

Question

In my quest for learning Objective-C, I have created the common FizzBuzz code in it. This is due to a suggestion/challenge from @nhgrif to re-implement anything he does in Swift in Objective-C.

#import <Foundation/Foundation.h>

int main(int argc, const char* argv[])
{
    @autoreleasepool
    {
        for (int i = 1; i <= 100; ++i)
        {
            if (i % 15 == 0) NSLog(@"FizzBuzz");
            else if (i % 3 == 0) NSLog(@"Fizz");
            else if (i % 5 == 0) NSLog(@"Buzz");
            else NSLog(@"%d", i);
        }
    }
}

Answer 1

I'd probably code the logic inside the loop this way:

if (i % fizz_divisor == 0) {        
    if (i % buzz_divisor == 0) {
        NSLog(@"FizzBuzz");
    } else {
        NSLog(@"Fizz");
    }
} else if (i % buzz_divisor == 0) { 
    NSLog(@"Buzz");
} else {
    NSLog(@"%d", i);
}

This invokes the % operator at most two times, rather than possibly three times. Of course now you have to define the constants fizz_divisor and buzz_divisor, but it is easier to make the program continue to work correctly if you later decide that the divisors should be something other than 3 and 5. It's also a pattern that's relatively easily extended if you want to implement a third divisor (e.g. to play "Fizz Buzz Ding").

Answer 2

I don't like that you've implemented logic in Main. Move the logic into a FizzBuzz method and call that from Main. (Even though it seems really silly for such a simple program, it's not best practice to have it there.)

I'm also not a fan of hard coding 15 as the "FizzBuzz" case. What if we decide that Fizz should be 2 and Buzz should be 5? Now 15 as your first case makes no sense and Mr. Maintainer has to figure out why 15 is a case at all. It's easier to understand and maintain if (i % 3 == 0 && i % 5 == 0) {.

This might be clearer if you consider 15,3, & 5 for what they really are. They're magic numbers and as such should be stored as constants. I don't know Obj-C, so I'll provide a pseudo code example of how I think it should be done.

const int fizz = 3
const int buzz = 5

if (value % fizz == 0 && value % buzz == 0) {
    Print("FizzBuzz")
} else if (value % fizz == 0) {
    Print("Fizz")
} else if (value % buzz == 0) {
    Print("Buzz")
} else {
    Print(value)
}

This allows us to easily update the conditions.

Or alternatively (thanks to Nhgrif and David K)

int fizzbuzz = lcm(fizz,buzz);
for (value = 1; value <= 100; value++) { 
    if (value % fizzbuzz == 0) {
        Print("FizzBuzz")
    } 
    //etc.
}

Where lcm() is some function that returns the lowest common multiple. At that point we may be overthinking the problem, but it does result in a robust solution.

Answer 3

You have no curly braces!

You should be sure to include curly braces. For one, it prevents mistakes when you want to go back and add some logic into one of these if statements in the future, and for two, since we're talking about Objective-C and you're almost certainly using Xcode, it makes the code collapsable.

---

I know, for some reason, that main drops its opening curly brace to the next line. I don't know why, and usually, I could care less because honestly, we spend almost no time in main when developing iOS/OSX projects, however... by default the @autoreleasepool leaves its opening brace on the same line, and the autocomplete on for statements do this as well.

It's a style thing, yes, but in Objective-C, we prefer the opening brace on the same line. And there's actually a good reason for this... If you notice in the left-hand margin of your code, just to the right of the line numbers, there are differing shades of gray. Clicking these shades of gray collapses different sections of your code.

Compare the difference of collapsing a for loop with the brace on the same line versus the brace on it's own line. To me, the latter looks silly and it takes up an extra line!

So all in all this looks a bit better:

int main(int argc, const char* argv[]) {
    @autoreleasepool {
        for (int i = 1; i <= 100; ++i) {
            if (i % 15 == 0) {
                NSLog(@"FizzBuzz");
            } else if (i % 3 == 0) {
                NSLog(@"Fizz");
            } else if (i % 5 == 0) { 
                NSLog(@"Buzz");
            } else {
                NSLog(@"%d", i);
            }
        }
    }
}

Answer 4

I'm no Objective-C expert, but it seems to me that since you are using neither argc nor argv that you could use int main(void).

Also, you could alternatively store the results of (i % 3 == 0) and (i % 5 == 0) and use those within the if statement.

It also seems that a return 0 might be needed at the end of main().

Answer 5

If we want to sacrifice readability in favor of performance, we might like something like this:

const int kFizz = 3;
const int kBuzz = 5;

for (int i = 1; i <= 100; ++i) {
    NSLog(@"%@",
        i % kFizz ? (i % kBuzz ? @"FizzBuzz" : @"Fizz") :
                    (i % kBuzz ? @"Buzz"     : @(i))
    );
}

The key to understanding this solution is that the %@ placeholder calls the description method on whatever argument it is sent. With NSString, the description is exactly what you'd expect. The tricky part here is @(i). The @(i) is modern Objective-C syntax which instantiates an NSNumber object, and calling the description method on an NSNumber object is almost equivalent to using the %i string format placeholder for an integer.