Raindrops in Java

Problem Statement:

Write a program that converts a number to a string, the contents of which depends on the number's prime factors.

• If the number contains 3 as a prime factor, output 'Pling'.
• If the number contains 5 as a prime factor, output 'Plang'.
• If the number contains 7 as a prime factor, output 'Plong'.
• If the number does not contain 3, 5, or 7 as a prime factor, just pass the number's digits straight through.

Code:

public class Raindrops {

private Raindrops() {}

public static String convert(int number) {
// Pre-condition.
if (number < 0) {
throw new IllegalArgumentException("Input cannot be negative.");
}
String result = "";
for (Raindrop drop : Raindrop.values()) {
if (drop.hasPrimeFactor(number)) {
result += drop.toString();
}
}
//if (result % 3 == 0)
//result += "Pling";
//if (number % 5 == 0)
//result += "Plang";
//if (number % 7 == 0)
//result += "Plong";
if (result.isEmpty()) {
result = "" + number;
}
checkPostCondition(result, number);
return result;
}

private static void checkPostCondition(String result, int number) {
assert(result.contains("Pling") ||
result.contains("Plang") ||
result.contains("Plong") ||
result.contains("" + number));
}

private enum Raindrop {
Pling(3),
Plang(5),
Plong(7);

private final int primeFactor;

private Raindrop(int primeFactor) {
this.primeFactor = primeFactor;
}

public boolean hasPrimeFactor(int number) {
return number % primeFactor == 0;
}
}
}


Test Suite:

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

import java.util.Arrays;
import java.util.Collection;

import static org.junit.Assert.assertEquals;

@RunWith(Parameterized.class)
public class RaindropsTest {

private int input;
private String expectedOutput;

@Parameters
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][]{
// Non-primes
{1, "1"},
{52, "52"},
{12121, "12121"},

// Numbers with 3 as a prime factor
{3, "Pling"},
{6, "Pling"},
{9, "Pling"},

// Numbers with 5 as a prime factor
{5, "Plang"},
{10, "Plang"},
{25, "Plang"},

// Numbers with 7 as a prime factor
{7, "Plong"},
{14, "Plong"},
{49, "Plong"},

// Numbers with multiple activating prime factors
{15, "PlingPlang"},
{21, "PlingPlong"},
{35, "PlangPlong"},
{105, "PlingPlangPlong"},
});
}

public RaindropsTest(int input, String expectedOutput) {
this.input = input;
this.expectedOutput = expectedOutput;
}

@Test
public void test() {
assertEquals(expectedOutput, Raindrops.convert(input));
}
}


Notes:

Although the solution was quite simple (see my commented code), I am trying to push myself and experiment with program correctness and flexibility, hence the final solution may seem over-engineered.

Reference

Post Condition

The post condition would pass even if the result is something like "PlingPlongNUMBER". (If I understood correctly, then the result should be either NUMBER or any combination of Pling/Plong/Plang, but never both.) Therefore, I suggest a post condition like the following (not tested!):

assert(((result.contains("Pling") || result.contains("Plang") || result.contains("Plong")) && !result.contains("" + number)) ||
(result.contains("" + number) && (!result.contains("Pling") && !result.contains("Plang") && !result.contains("Plong")))));


OOP changes

Since you are exercising OOP (and don't mind some over-engineering ;)) , you could consider adding another RainDrop, for dealing with the case that no prime factor matches (e.g. RainDrop(-1)). This would require the following modifications

• Rename hasPrimeFactor to something reflect more accurately what it does (e.g. processNumber).

• For the enum -1, processNumber would check that the parameter is NOT divisible by either 3, 5 or 7, and return the number as a string if that is the case.

• Possible performance improvement: you might want to build a cache for divisibility with 3, 5 and 7, in case you are worried that modulo is calculated twice for each dividend. (E.g. cache table for 3 can be a HashMap, that for each already seen number tells if it is divisible with 3 or not. Same for 5 and 7.) Caveat: I did not verify that doing the lookup in the HashMap (let alone lookup + storage!) is faster than doing the modulo division, so you might actually lose performance in this way. The idea is rather to experiment with a way of caching, in case you were doing a really expensive operation.

As I said above, I'm not saying that this suggested change is necessarily better than the code you have now (in fact it is a bit more over-engineered). It is rather a way of exploring how to further OOP-fy your code.

Test Code

Let me first say, that I find it a really positive thing that you write your code with testing in mind, and add unit tests to cover (almost) all the cases. That said, some remarks:

• Corner cases: I suggest adding tests for 0 and -1 as well. (Maybe -1 is tricky, since you expect an exception, all the same it is worth the effort.)

• Comment about // Non-primes: while technically correct (the input numbers are non-primes), the point is that the numbers are not divisible by 3, 5 or 7, not that they are non-primes. So, I suggest updating the comment accordingly.