I know this is somewhat of a subjective question, but I could find no answer to it. I'll give details, though:
I have a class that has a lot of configuration methods, so that you can run a specific algorithm with very different settings. After you properly configure it, you can just call a method "generate()", and it returns a filled 2d array. That's the method that is hard to test. It is that hard because it returns a heightmap, which contains randomness and a large amout of data. Generally, I would split it, but that would break encapsulation, because in practice the data and the steps of the algorithm changes for the same reason, and no one would use one separeted from the other. I know I can mock the randomness (which I'm actually doing), but in either way it is too much data to be practical to writes tests for (for instance a mere 32 by 32 map would require 1024 assertions on a single test). Besides that, generating a big map can take up to 3 or 4 seconds, which is obviously not good in a test ambient.
Also, I tried first to separate the data from the algorithm, and make the algorithm pure functional (no data, only arguments). While this was testable (because I split the algorithm), this lead to lots of arguments being passed all around and a rather procedural-looking code.
So I'm not sure how to procede here. Splitting the class makes it easier to test and to understand, but also breaks encapsulation really bad. I would appreciate any thoughs on the subject, either practical or theoretical.
Thanks in advance.
Edit: This is the code that I wrote and is procedural-like.
JavaRandom, Attenuation and RandomNumberProcessor are classes that I should have kept together.
This is the data used by the algorithm:
package noise;
import noise.attenuation.DefaultAttenuation;
public final class DiamondSquareSettings {
public static final class Builder {
private static final double NEVER_ASSIGNED = 1.1;
private static final long DEFAULT_RNG_SEED = 33609606l;
private static final JavaRandom DEFAULT_RANDOM = new JavaRandom(DEFAULT_RNG_SEED);
private static final double DEFAULT_RANGE = 1;
private static final Attenuation defaultAttenuation = new DefaultAttenuation(0.7);
private Random builderRandom = DEFAULT_RANDOM;
private final double[] builderSeeds = {NEVER_ASSIGNED , NEVER_ASSIGNED , NEVER_ASSIGNED , NEVER_ASSIGNED};
private double builderRange = DEFAULT_RANGE;
private Attenuation builderAttenuation = defaultAttenuation;
public Builder attenuation(final Attenuation attenuation) {
validateAttenuation(attenuation);
builderAttenuation = attenuation;
return this;
}
public DiamondSquareSettings build(final int side) {
validateSize(side);
return new DiamondSquareSettings(side, builderRandom, builderSeeds, builderRange, builderAttenuation);
}
public Builder random(final long seed) {
return random(new JavaRandom(seed));
}
public Builder randomRange(final double range) {
validateRange(range);
builderRange = range;
return this;
}
public Builder seed(final int seedPosition, final double seedValue) {
validateSeeds(seedPosition, seedValue);
builderSeeds[seedPosition] = seedValue;
return this;
}
Builder random(final Random random) {
validateRandom(random);
builderRandom = random;
return this;
}
private void validateAttenuation(final Attenuation attenuation) {
if (attenuation == null)
throw new IllegalArgumentException("attenuation == null!");
}
private void validateRandom(final Random random) {
if (random == null)
throw new IllegalArgumentException("random == null!");
}
private void validateRange(final double range) {
if (range < 0)
throw new IllegalArgumentException("range < 0" + "\nrange: " + range);
}
private void validateSeeds(final int seedPosition, final double seedValue) {
if (seedValue > 1 || seedValue < 0)
throw new IllegalArgumentException("Invalid seed" + "\nseedPosition: " + seedPosition + " seed: "
+ seedValue);
}
private void validateSize(final int side) {
final double doubleExpoent = Math.log(side - 1) / Math.log(2);
final int integerExpoent = (int) doubleExpoent; //TODO simplify more
if (doubleExpoent != integerExpoent)
throw new IllegalArgumentException("side is not a (power of two) plus one" + "\nside: " + side);
}
}
private final int side;
private final Random random;
private final double[] seeds;
private final double range;
private final Attenuation attenuation;
private DiamondSquareSettings(final int side, final Random random, final double[] seeds, final double range,
final Attenuation attenuation) {
this.side = side;
this.random = random;
this.seeds = randomizeNeverAssignedSeeds(random, seeds, range);
this.range = range;
this.attenuation = attenuation;
}
public String getAttenuationInfo() {
return attenuation.toString();
}
public String getRandomInfo() {
return random.toString();
}
public double getRandomRange() {
return range;
}
public double getSeed(final int seedPosition) {
return seeds[seedPosition];
}
public int getSide() {
return side;
}
Attenuation getAttenuation() {
return attenuation;
}
Random getRandom() {
return random;
}
private double[] randomizeNeverAssignedSeeds(final Random random, final double[] seeds, final double range) {
final double[] properlyRandomizingSeeds = seeds;
for (int i = 0 ; i < seeds.length ; i++)
if (seeds[i] == Builder.NEVER_ASSIGNED)
properlyRandomizingSeeds[i] = random.nextRandomValueInside(range);
return properlyRandomizingSeeds;
}
}
This is the bulk of the algorithm:
package noise;
public final class DiamondSquare {
public static Noise generate(final DiamondSquareSettings settings) {
validateSettings(settings);
final DiamondSquareStep stepper = new DiamondSquareStep();
double[][] noiseArray = new double[settings.getSide()][settings.getSide()];
setupSeeds(settings, noiseArray);
final RandomNumberProcessor processor =
new RandomNumberProcessor(settings.getRandomRange(), settings.getRandom(), settings.getAttenuation());
noiseArray = processDiamondSquare(settings, stepper, noiseArray, processor);
return new Noise(noiseArray);
}
private static double[][] processDiamondSquare(final DiamondSquareSettings settings,
final DiamondSquareStep stepper, double[][] noiseArray, final RandomNumberProcessor processor) {
int stepSize = settings.getSide() - 1;
while (stepSize >= 1) {
noiseArray = stepper.diamondSquare(stepSize, noiseArray, processor);
processor.nextStage();
stepSize /= 2;
}
return noiseArray;
}
private static void setupSeeds(final DiamondSquareSettings settings, final double[][] noiseArray) {
noiseArray[0][0] = settings.getSeed(0);
noiseArray[settings.getSide() - 1][0] = settings.getSeed(1);
noiseArray[0][settings.getSide() - 1] = settings.getSeed(2);
noiseArray[settings.getSide() - 1][settings.getSide() - 1] = settings.getSeed(3);
}
private static void validateSettings(final DiamondSquareSettings settings) {
if (settings == null)
throw new IllegalArgumentException("settings == null!");
}
}
This are the other steps of the algorithm:
package noise;
final class DiamondSquareResolver {
private static final double OUT_OF_RANGE = 0; //TODO this code smells. It alters slightly the final result. Should be gone.
private static final double DISAMBIGUATION = 0.00001;
public double diamondStep(final int i, final int j, final int halfStep, final double[][] noise,
final RandomNumberProcessor processor) {
final double left = initializeLeft(i - halfStep, j, noise);
final double right = initializeRight(i + halfStep, j, noise);
final double top = initializeTop(i, j - halfStep, noise);
final double bot = initializeBot(i, j + halfStep, noise);
final int divisor = processDivisor(left, right, top, bot);
return (left + right + top + bot) / divisor + processor.generateNumber();
}
public double squareStep(final int i, final int j, final int halfStep, final double[][] noise,
final RandomNumberProcessor processor) {
final double topLeft = noise[i - halfStep][j - halfStep];
final double topRight = noise[i + halfStep][j - halfStep];
final double bottomLeft = noise[i - halfStep][j + halfStep];
final double bottomRight = noise[i + halfStep][j + halfStep];
return (topLeft + topRight + bottomLeft + bottomRight) / 4 + processor.generateNumber();
}
private double initializeBot(final int i, final int bottomCoordinate, final double[][] noise) {
final int height = noise[0].length;
if (! (bottomCoordinate >= height))
return validatedPosition(noise[i][bottomCoordinate]);
else
return OUT_OF_RANGE;
}
private double initializeLeft(final int leftCoordinate, final int j, final double[][] noise) {
if (! (leftCoordinate < 0))
return validatedPosition(noise[leftCoordinate][j]);
else
return OUT_OF_RANGE;
}
private double initializeRight(final int rightCoordinate, final int j, final double[][] noise) {
final int width = noise.length;
if (! (rightCoordinate >= width))
return validatedPosition(noise[rightCoordinate][j]);
else
return OUT_OF_RANGE;
}
private double initializeTop(final int i, final int topCoordinate, final double[][] noise) {
if (! (topCoordinate < 0))
return validatedPosition(noise[i][topCoordinate]);
else
return OUT_OF_RANGE;
}
private int processDivisor(final double ... sides) { //TODO remove varagrs argument, as it is not proper.
int divisor = 4;
for (final double side : sides)
if (side == OUT_OF_RANGE)
divisor--;
return divisor;
}
private double validatedPosition(final double value) {
return value != OUT_OF_RANGE ? value : value + DISAMBIGUATION;
}
}
final class DiamondSquareStep {
public double[][] diamondSquare(final int step, final double[][] noise, final RandomNumberProcessor processor) {
final double[][] steppedNoise = copyNoise(noise);
final DiamondSquareResolver solver = new DiamondSquareResolver();
final int halfStep = step / 2;
performSquareSteps(step, steppedNoise, processor, solver, halfStep);
performDiamondSteps(step, steppedNoise, processor, solver, halfStep);
return steppedNoise;
}
private double[][] copyNoise(final double[][] noise) {
final double[][] steppedNoise = new double[noise.length][noise[0].length];
for (int i = 0 ; i < noise.length ; i++)
for (int j = 0 ; j < noise[0].length ; j++)
steppedNoise[i][j] = noise[i][j];
return steppedNoise;
}
private void performDiamondSteps(final int step, final double[][] noise, final RandomNumberProcessor processor,
final DiamondSquareResolver solver, final int halfStep) {
for (int i = 0 ; i < noise.length ; i += step)
for (int j = 0 ; j < noise[0].length ; j += step) {
if (i + halfStep < noise.length)
noise[i + halfStep][j] = solver.diamondStep(i + halfStep, j, halfStep, noise, processor);
if (j + halfStep < noise[0].length)
noise[i][j + halfStep] = solver.diamondStep(i, j + halfStep, halfStep, noise, processor);
}
}
private void performSquareSteps(final int step, final double[][] noise, final RandomNumberProcessor processor,
final DiamondSquareResolver solver, final int halfStep) {
for (int i = halfStep ; i < noise.length ; i += step)
for (int j = halfStep ; j < noise[0].length ; j += step)
noise[i][j] = solver.squareStep(i, j, halfStep, noise, processor);
}
}
It's absolutely acceptable to divide a class up to satisfy the Single Responsibility Principle. The test for whether you're violating this principle can be summarized as:
One great benefit of satisfying the principle is that, in general, your unit will have less in it that needs to be tested, making testing easier. But there are other reasons. A class with a single responsibility will only be changed for one reason, and so it is much more maintainable and stable.
However, the way that you go about it is also important. Unless an IDE is doing the splitting up for you algorithmically, you're liable to introduce bugs in the refactoring. So it's usually a good idea to have the class under test before you refactor it, and then after you've done the refactoring (and everything still passes) divide up the tests correspondingly.