Skip to main content
added 383 characters in body
Source Link
K.H.
  • 2.7k
  • 7
  • 25
public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

Constructing the string from pieces

You can reduce one if by removing "else" conditions. Downside is, that you need to add another if in the end for the return values so probably not better, but should be mentioned:

public String pickOneV2(int input) {
    var sb = new StringBuilder();
    if (input % 3 == 0) {
        sb.append("fizz");
    }
    if (input % 5 == 0) {
        sb.append("buzz");
    } 

    if (sb.length() == 0) {
        return String.valueOf(input); 
    }        
    return sb.toString();
}
public String pickOneV2pickOneV3(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}
public String pickOneV2pickOneV4(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    return fizzbuzzMap.entrySet().stream().filter(
            entry -> input % entry.getKey() == 0
    ).map(Map.Entry::getValue).findFirst().orElse(String.valueOf(input));
}
public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 
public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}
public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    return fizzbuzzMap.entrySet().stream().filter(
            entry -> input % entry.getKey() == 0
    ).map(Map.Entry::getValue).findFirst().orElse(String.valueOf(input));
}
public String pickOne(int input) {

    if (input % 15 == 0) {
        return "fizzbuzz";
    } else if (input % 3 == 0) {
        return "fizz";
    } else if(input % 5 == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

Constructing the string from pieces

You can reduce one if by removing "else" conditions. Downside is, that you need to add another if in the end for the return values so probably not better, but should be mentioned:

public String pickOneV2(int input) {
    var sb = new StringBuilder();
    if (input % 3 == 0) {
        sb.append("fizz");
    }
    if (input % 5 == 0) {
        sb.append("buzz");
    } 

    if (sb.length() == 0) {
        return String.valueOf(input); 
    }        
    return sb.toString();
}
public String pickOneV3(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}
public String pickOneV4(int input) {
    return fizzbuzzMap.entrySet().stream().filter(
            entry -> input % entry.getKey() == 0
    ).map(Map.Entry::getValue).findFirst().orElse(String.valueOf(input));
}
stream api example
Source Link
K.H.
  • 2.7k
  • 7
  • 25

If simplification

First if can be simplified to input % 15. Basically you are checking for number divisible by 3 and 5, so you can just check for 15. I agree with using brackets even here as mentioned by @Captain Man

public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

Data-oriented approach

With all if conditions being in the same format, you can extract this into a data structure and simplify your code even more. I kept the map in the same method for simplicity reasons, but in reality it should be class property or a constant. This is the most extensible and clean I can think of, and you can easily modify this functionality purely by modifying the data without touching your code.

public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}

Using stream API

Slightly nicer version (imho) using streams:

public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    return fizzbuzzMap.entrySet().stream().filter(
            entry -> input % entry.getKey() == 0
    ).map(Map.Entry::getValue).findFirst().orElse(String.valueOf(input));
}

If simplification

First if can be simplified to input % 15. Basically you are checking for number divisible by 3 and 5, so you can just check for 15. I agree with using brackets even here as mentioned by @Captain Man

public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

Data-oriented approach

With all if conditions being in the same format, you can extract this into a data structure and simplify your code even more. I kept the map in the same method for simplicity reasons, but in reality it should be class property or a constant. This is the most extensible and clean I can think of, and you can easily modify this functionality purely by modifying the data without touching your code.

public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}

If simplification

First if can be simplified to input % 15. Basically you are checking for number divisible by 3 and 5, so you can just check for 15. I agree with using brackets even here as mentioned by @Captain Man

public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

Data-oriented approach

With all if conditions being in the same format, you can extract this into a data structure and simplify your code even more. I kept the map in the same method for simplicity reasons, but in reality it should be class property or a constant. This is the most extensible and clean I can think of, and you can easily modify this functionality purely by modifying the data without touching your code.

public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}

Using stream API

Slightly nicer version (imho) using streams:

public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    return fizzbuzzMap.entrySet().stream().filter(
            entry -> input % entry.getKey() == 0
    ).map(Map.Entry::getValue).findFirst().orElse(String.valueOf(input));
}
added 886 characters in body
Source Link
K.H.
  • 2.7k
  • 7
  • 25

If simplification

First if can be simplified to input % 15. Basically you are checking for number divisible by 3 and 5, so you can just check for 15. I agree with using brackets even here as mentioned by @Captain Man

public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

Data-oriented approach

With all if conditions being in the same format, you can extract this into a data structure and simplify your code even more. I kept the map in the same method for simplicity reasons, but in reality it should be class property or a constant. This is the most extensible and clean I can think of, and you can easily modify this functionality purely by modifying the data without touching your code.

public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}

If simplification

First if can be simplified to input % 15. Basically you are checking for number divisible by 3 and 5, so you can just check for 15. I agree with using brackets even here as mentioned by @Captain Man

public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

If simplification

First if can be simplified to input % 15. Basically you are checking for number divisible by 3 and 5, so you can just check for 15. I agree with using brackets even here as mentioned by @Captain Man

public String pickOne(int input) {

    if ((input % 15) == 0) {
        return "fizzbuzz";
    } else if ((input % 3) == 0) {
        return "fizz";
    } else if((input % 5) == 0) {
        return "buzz";
    }

    return String.valueOf(input);
} 

Data-oriented approach

With all if conditions being in the same format, you can extract this into a data structure and simplify your code even more. I kept the map in the same method for simplicity reasons, but in reality it should be class property or a constant. This is the most extensible and clean I can think of, and you can easily modify this functionality purely by modifying the data without touching your code.

public String pickOneV2(int input) {
    var fizzbuzzMap = new TreeMap<Integer, String>(Comparator.reverseOrder());
    fizzbuzzMap.put(3, "fizz");
    fizzbuzzMap.put(5, "buzz");
    fizzbuzzMap.put(15, "fizzbuzz");

    for (var index : fizzbuzzMap.keySet()) {
        if (input % index == 0) {
            return fizzbuzzMap.get(index);
        }
    }

    return String.valueOf(input);
}
Source Link
K.H.
  • 2.7k
  • 7
  • 25
Loading