-2
#include <iostream>
using namespace std;

int f[33] = {3, 6, 9, 12, 15, 18, 21, 24, 27, 30, 33, 36, 39, 42, 45, 48, 51, 54, 
             57, 60, 63, 66, 69, 72, 75, 78, 81, 84, 87, 90, 93, 96, 99};

int b[20] = {5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60, 65, 70, 75, 80, 85, 
             90, 95, 100};


int main (){

  for (int x=100; x >= 1; x-- ){

    if (x == f){
        cout << "fizz" << endl;
    } else {
        if(x ==b){
            cout << "buzz" << endl;
        }else{
            if(x==f & x==b){
                cout << "fizzbuzz" << endl;

            }else{
                cout << x << endl;
            }
        }
    }
  }
}

I am still learning, so this may not be the best way to solve this problem. I just want to know wahts wrong with this code. Thanks

StuartLC
  • 104,537
  • 17
  • 209
  • 285
daesad
  • 55
  • 6
  • Is something wrong with the code? What's wrong? – Drew Dormann May 31 '15 at 04:48
  • 3
    This is why fizzbuzz is such a great interview question – M.M Jun 13 '15 at 12:13
  • @daesad the first thing to do is to read the error messages your compiler generated when you compiled this code. (If you didn't get any , then look up how to enable errors and warnings in your compiler). Understand what those errors mean and that will help you start to understand what is wrong with your program. (Hint: `==` means "equals", not "is a member of". There is no "is a member of" operator for C-style arrays in C++.) – M.M Jun 13 '15 at 12:16

4 Answers4

2

As others have pointed out, you've precomputed your multiples of 3 and 5 in arrays, but then doing direct comparisons between an int and these arrays - this will always fail (ISO C++ forbids comparison between pointer and integer). If you persist with precomputed arrays, you could use std::find or std::any_of to check if either array contains the current number.

However, you would likely gain more credibility if you also included knowledge of how to determine whether a number is divisible by 3 or 5 in your code, rather than pre-populate the multiples of 3 and 5. This is done with the modulo operator, %. Any number % x will return zero if it is naturally divisible by x.

There's another logical flaw in your code. In order to be divisible by both 3 and 5 (i.e. 15, since 3 and 5 are both primes), you will need to change the order of precedence of your checks such that the check for 15 is done first, otherwise you will never reach the fizbuzz branch (since the 3 and 5 branches would also be hit, instead).

Fizzbuzz is usually done incrementally from 1 to 100, but here's your original 'count down' fizzbuzz rewritten:

for (int x=100; x >= 1; x--){
   bool isDiv3 = x % 3 == 0;
   bool isDiv5 = x % 5 == 0;
   if (isDiv3 && isDiv5){
      cout << "fizzbuzz" << endl;
   } else if (isDiv5) {
      cout << "buzz" << endl;
   } else if (isDiv3) {
      cout << "fizz" << endl;
   } else {
       cout << x << endl;
   }
}

It's also possible to eliminate one of the if branches, by running the printed fizz and buzz into eachother on a factor of 15, although this isn't necessarily as easy to read:

for (int x=100; x >= 1; x--){
   bool isDiv3 = x % 3 == 0;
   bool isDiv5 = x % 5 == 0;
   if (isDiv3) {
      cout << "fizz";
   } 
   if (isDiv5) {
      cout << "buzz";
   } 
   if (!isDiv3 && !isDiv5)
       cout << x;
   }
   cout << endl;
}
StuartLC
  • 104,537
  • 17
  • 209
  • 285
1

The error with your code is you can't compare integer and pointer , What you are trying to do is to find if x is in f or if x is in b or both . But why do yo have to do that , you know the properties governing the sets f and b which are simply "%3==0" , "%5==0" .So you can do something pretty easy like

 #include <iostream>
using namespace std;

int main (){

for (int x=100; x >= 1; x-- ){
if(x%3==0)        cout<<"Fizz";
if(x%5==0)        cout<<"Buzz";
else if(x%3 !=0)  cout <<x;
 cout<<endl;
}

}
0

x is an int, while f is an array. You cannot compare them this way:

if (x == f){

If your technique is to check whether x is in the array f, I suggest, you have to check for each value in f, like

 if(x == f[i++]){

where i is an index used to traverse the f array.

Also, you might consider evaluating the condition of x in both f and b before their individual evaluation.

Ayushi Jha
  • 4,003
  • 3
  • 26
  • 43
0

x is an integer whereas f and b are arrays of integers. If you want to test membership of the content of your variable x in the arrays f and b you probably want to define your own function to check it.

int is_in(int item, int[] list){
    for(i = 0; i < sizeof(list) / sizeof(struct list); i++){
        if(item==list[i]) return 1;
    }
    return 0;
}

And then change your conditions to if(is_in(x,b))

Josep Valls
  • 5,483
  • 2
  • 33
  • 67