2

Attempting a FizzBuzz recursive solution in Java to return a list of Strings with n iterations. For example, n = 4 should output ["1","2","Fizz", 4]. However, with my current code the output is just ["4"]. Why is my solution not executing the recursive function? Any other critiques are appreciated!

class Solution {
public List<String> fizzBuzz(int n) {

    //create variable to return list of strings
    List<String> fbList = new ArrayList<String>();

    //base case 1
    if(n == 0){
        fbList.add(Integer.toString(0));
    }

    //base case 2
    else if(n == 1){
        fbList.add(Integer.toString(1));
    }    

    //OW take n and begin reducing recursively from, n - 1
    else{
        if(n % 3 == 0){
            fbList.add("Fizz");
        }
        else if(n % 5 == 0){
            fbList.add("Buzz");
        }
        else if((n % 3 == 0) && (n % 5 == 0)){
            fbList.add("FizzBuzz");
        }
        else{
            fbList.add(Integer.toString(n));
        }
        //recursive function call
        fizzBuzz(n - 1);
    }
    return fbList;
    }
}
cdlane
  • 40,441
  • 5
  • 32
  • 81
alexjs000
  • 85
  • 9
  • 2
    It looks like you need to learn to use a debugger. Please help yourself to some [complementary debugging techniques](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). If you still have issues afterwards, please feel free to come back with a more specific question. – Joe C Dec 09 '18 at 22:00
  • You might want to run `else if((n % 3 == 0) && (n % 5 == 0)){ fbList.add("FizzBuzz"); }` first in your `if-statement`. – SedJ601 Dec 09 '18 at 22:10

2 Answers2

2

The problem is that with every recursive call, a new List is created. You return the list but:

fizzBuzz(n - 1);

You are ignoring the return value of the recursive calls. To fix this you can do:

fbList.addAll(0, fizzBuzz(n - 1));

Which will utilize the method addAll to add all the elements returned by the recursive calls. This returns:

[1, 2, Fizz, 4]

However this is rather expensive for an ArrayList. You could change this to a LinkedList, which would allow for linear time adding.


Also you if/else if/else chain is out of order. if((n % 3 == 0) && (n % 5 == 0)) should be before if(n % 3 == 0) and if(n % 5 == 0). Otherwise it will always enter the if(n % 5 == 0) or if(n % 3 == 0):

if((n % 3 == 0) && (n % 5 == 0)){
    fbList.add("FizzBuzz");
}
else if(n % 3 == 0){
    fbList.add("Fizz");
}
else if(n % 5 == 0){
    fbList.add("Buzz");
}
GBlodgett
  • 12,704
  • 4
  • 31
  • 45
  • That allows me to output the values ["4", "Fizz", "2", "1"]. I guess this solution would only output in a descending order. Is there a way to output in an ascending order? – alexjs000 Dec 09 '18 at 22:08
  • @alexjs000 Append it in reverse order or reverse the entire result. – David G Dec 09 '18 at 22:19
  • Would like to keep the complexity at O(n), won't using a function to reverse the entire order make it O(n^2)? – alexjs000 Dec 09 '18 at 22:22
  • @alexjs000 You can use the overloaded `addAll` method to add it to the beginning – GBlodgett Dec 09 '18 at 22:22
  • @alexjs000 No, prepending at each step (n times in total) to a data structure (ArrayList) that is not designed for that operation (i.e. it's linear in the number of elements already contained) would make it _O(n^2)_. `LinkedList` is linear in prepend though. – Nándor Előd Fekete Dec 09 '18 at 23:43
0

Think simple when working with recursion, i.e. let recursion do the work. If your recursion is counting down but you want the list to come out ascending, add everything else first then add the thing you're working on:

import java.util.*;

public class Solution {

    public static List<String> pattern = Arrays.asList("FizzBuzz", "", "", "Fizz", "", "Buzz", "Fizz", "", "", "Fizz", "Buzz", "", "Fizz", "", "");

    public static List<String> fizzBuzz(int n) {

        List<String> fbList;

        if (n > 0) {
            fbList = fizzBuzz(n - 1);
            String string = pattern.get(n % pattern.size());
            fbList.add(string.isEmpty() ? Integer.toString(n) : string);
        } else {
            fbList = new ArrayList<String>();
        }

        return fbList;
    }

    public static void main(String[] args) {
        System.out.println(fizzBuzz(Integer.parseInt(args[0])));
    }
}

OUTPUT

> java Solution 35
[1, 2, Fizz, 4, Buzz, Fizz, 7, 8, Fizz, Buzz, 11, Fizz, 13, 14, FizzBuzz, 16, 17, Fizz, 19, Buzz, Fizz, 22, 23, Fizz, Buzz, 26, Fizz, 28, 29, FizzBuzz, 31, 32, Fizz, 34, Buzz]
> 
cdlane
  • 40,441
  • 5
  • 32
  • 81
  • This is a very inefficient `O(n^2)` as you're copying the entire list again at each step. – Lie Ryan Dec 10 '18 at 03:03
  • Not necessarily. There are a number of more efficient options: you can use LinkedList which have an efficient prepend, you can collect into ArrayList in reverse order then reverse the array in O(n), or you can pass in the result ArrayList as an argument. All of those approaches will be `O(n)`. – Lie Ryan Dec 10 '18 at 03:15
  • @LieRyan, thanks, I've modified the solution to just uses a single ArrayList. – cdlane Dec 10 '18 at 03:18