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

FizzBuzz in Objective-C

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

Problem

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 

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);
        }
    }
}

Solution

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.

Code Snippets

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)
}
int fizzbuzz = lcm(fizz,buzz);
for (value = 1; value <= 100; value++) { 
    if (value % fizzbuzz == 0) {
        Print("FizzBuzz")
    } 
    //etc.
}

Context

StackExchange Code Review Q#56844, answer score: 9

Revisions (0)

No revisions yet.