2
\$\begingroup\$

Something I haven't often seen in FizzBuzz implementation is a focus on making it flexible and maintainable. I have attempted to create a version that prioritizes these, such as by making it easier to change the conditions and add more conditions. What else could I do to work towards these criteria?

public class V3_ifFunc
{
   public static void main(String[] args)
   {
       String out;
       int i;
       for(i=0; i<=100; i++)
       {
           out = "";
           if(divisible(i, 3)){
               out += "Fizz";
           }
           if(divisible(i, 5)){
               out += "Buzz";
           }
           if(out.equals("")){
               out += Integer.toString(i);
           }
           System.out.println(out);
       }
   }
   public static boolean divisible(int i, int by)
   {
       if(i%by==0){
           return true;
       }else{
           return false;
       }
   }
}
\$\endgroup\$
3
  • 3
    \$\begingroup\$ I'd argue that FizzBuzz is so small and simple that as long as you don't overengineer things almost any straightforward implementation will be flexible and maintainable. I don't think there's much to learn from this exercise, other than some basic programming constructions such as loops and if/else statements. \$\endgroup\$ – Pieter Witvoet Sep 13 '17 at 13:29
  • 2
    \$\begingroup\$ I would recommend this as a sanity check: github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition \$\endgroup\$ – Erich Sep 13 '17 at 19:34
  • \$\begingroup\$ This article may be relevant as well. Although written for Ruby, many of the concepts apply to Java. \$\endgroup\$ – JS1 Sep 16 '17 at 23:35
2
\$\begingroup\$
  • The class should be called FizzBuzz instead of V3_ifFunc.
  • The variable i must not be visible outside the for loop.
  • The variable out must only be visible inside the for loop.
  • It is common style to write spaces after for and if.
  • Instead of out.equals(""), it is simpler to write out.isEmpty().
  • If out is empty, there's no point in using the += operator. It's also easier to just do System.out.println(i) instead of first converting it into a string, then concatenating it with an empty string and then printing the result.
  • The method divisible is a useless abstraction. For the usual arithmetic rules, the % operator is good enough. It would only make sense if you were to extend FizzBuzz to use modulo arithmetics in a prime field. Are you planning to do that?
  • The if (condition) return true; else return false can be written as return condition, which is much simpler.
\$\endgroup\$
2
\$\begingroup\$

You could make some sort of struct or class that stores the text printout and the number i should be divisible by. Then you just need to create as many instances of the class as you need and loop through them.

One other approach is to have a hashtable and store the keys in an array that you loop through. You can use whichever one you feel makes the most sense but the first method should technically use up less memory because you only store the text once for each condition. I might be wrong though, I don't really use Java.

Example (Not tested and not written by a Java developer):

class Condition {
    String printout;
    int div;
}

Condition[] conditions = { {"Fizz", 3}, {"Buzz", 5} };
int loops = 100;

public static void main() {
   for(int i = 0; i < loops; i++) {
       String printOut = "";
       for(int j = 0; j < conditions.length; i++) {
           if(i % conditions[j] == 0) {
              printOut += conditions[j].printout;
           }
       }
       if(printOut == "") {
          System.out.println(Integer.toString(i));
       } else {
          System.out.println(printOut);
       }
   }
}

Then if you want to you could wrap it in a function and call it with the conditions array and number of loops as parameters.

You can also shorten your divisible to:

public static boolean divisible(int a, int b) {
   return ((a % b) == 0);
}

But I think that you don't really need that function because you can just use the % operator and your code would be cleaner and quicker to understand. Just for future consideration.

Hope this answers your question.

\$\endgroup\$
8
  • \$\begingroup\$ Part of the idea of my program was that FizzBuzz would change automatically if I, say, change "Fizz" to "Fuzz" or %3 to %4. \$\endgroup\$ – Jay Sep 13 '17 at 12:25
  • \$\begingroup\$ @Jay the anwser fully supports that. \$\endgroup\$ – Timothy Truckle Sep 13 '17 at 12:56
  • \$\begingroup\$ In the answer, FizzBuzz is implemented as a separate Condition object - so, if I modified the Buzz object, FizzBuzz would be entirely unaffected. \$\endgroup\$ – Jay Sep 13 '17 at 13:17
  • \$\begingroup\$ Acutally you don't need a seperate condition for FizzBuzz in this example my mistake (also I edited the answer to reflect this), but if you continue the outer loop you would have to have a seperate condition. You can use labels to do this: link \$\endgroup\$ – Signekatt Sep 13 '17 at 13:48
  • \$\begingroup\$ This makes adding simple divide rules easier, but other kinds of rules and the interaction between rules are now harder to manage. I'd say that that makes it less flexible and harder to maintain. \$\endgroup\$ – Pieter Witvoet Sep 13 '17 at 13:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.