2
\$\begingroup\$

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

\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$

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.