Select two numbers from interlaced blocks of numbers

Question

Select one of two number selection strategies and select two numbers using that strategy.

The strategy is selected using an enum implementation of the strategy pattern.

Strategy one: Ordered Numbers are selected sequentially. The last number selected is persisted in the database and the next two numbers are selected.

Strategy two: Interlaced Numbers are selected from interlaced groups of numbers so that no two numbers are sequential and sufficiently apart to appear randomly selected. No number can be selected more than once.

Interlaced blocks of numbers are generated based on a block size and a minimum and maximum number. Numbers are selected from the begining of one block and the end of the other in ascending and descending order respectively.

Example: Total numbers 10000, starting at 1000 with a block size of 1000.

block 1: 1000 - 1999
block 2: 2000 - 2999
block 3: 3000 - 3999
block 4: 4000 - 4999
...

Note: the last block should end 10,000 not 9999.

Two numbers are chosen. One number is chosen from the beginning of block 1 and another from the end of block 3, this should continue until both blocks have been exhausted of numbers. Selection should start again from the beginning of block 2 and the end of block 4 until all numbers where exhausted. This should continue until all blocks have been used up.

Important points to take account of:

• There must be an even number of blocks

• The block size must be such that the number of blocks is even such that:

  ((endRange - startRange) + 1 / blockSize ) % 2 == 0;
  

I will not include the code for the ordered strategy as this is not the code a would like a review for. Below I include code for interlaced strategy.

This class represents the blocks of numbers in the interlaced strategy:

public class Block implements Comparable<Block> {

    public enum Status {
        LIVE,
        IN_USE,
        DEAD;
    }

    private Integer idx;
    private Integer startOfBlock;
    private Integer endOfBlock;
    private Integer previousNumber;
    private Status status;

    public int compareTo(Block block) {
        return this.getIdx().compareTo(block.getIdx());
    }

    // getters and setters removed for brevity

    }

The following classes represent the strategy selection:

    public interface Strategy<T> {
        List<T> execute(Config config);
    }



    public enum NumberStrategy {

        ORDERED(new OrderedStrategy()),
        INTERLACED(new InterlacedStrategy());

        private Strategy<Integer> strategy;

        NumberStrategy(Strategy<Integer> strategy) {
            this.strategy = strategy;
        }

        public Strategy<Integer> getStrategy() {
            return strategy;
        }
    }

    public class OrderedStrategy implements Strategy<Integer> {
        // implementation code omitted - see git repository
    }

For the purpose of this code review I have put the number block generation method in utility class. The generateBlocks method should generate all the blocks of numbers and arrange them in an interlaced manner as described above. They would then be persisted in the database a retrieved for each request for two numbers.

    public class Utils {

        public static List<Block> sortedBlocks = new ArrayList<>();

        /**
         * Make the blocks for the given raffle.
         */
        public static void generateBlocks(Config config) {

            sortedBlocks.clear();

            int possibleRange = (config.getLastNumber() - config.getInitialNumber()) + 1;

            double numberOfBlocks = possibleRange / config.getBlockSize();

            List<Block> blocks = new ArrayList<>();

            int currentBlockStart = config.getInitialNumber();
            int currentBlockEnd;

            for (int i = 1; i <= numberOfBlocks; i++) {
                currentBlockEnd = currentBlockStart + config.getBlockSize() - 1;

                // Last block end number will always be equal to the endRange number
                if (i == numberOfBlocks) {
                    currentBlockEnd = config.getLastNumber();
                }

                Block block = new Block(i, currentBlockStart, currentBlockEnd, null, Block.Status.LIVE);

                blocks.add(block);
                currentBlockStart = (currentBlockEnd + 1);
            }

            // Split chucks and order
            List<Block> listOddBlocks = blocks
                    .stream()
                    .filter(block -> block.getIdx() % 2 == 1)
                    .sorted(Block::compareTo)
                    .collect(Collectors.toList());

            List<Block> listEvenBlock = blocks
                    .stream()
                    .filter(block -> block.getIdx() % 2 == 0)
                    .sorted(Block::compareTo)
                    .collect(Collectors.toList());

            // Concatenate the two lists.
            sortedBlocks.addAll(listOddBlocks);
            sortedBlocks.addAll(listEvenBlock);

            // Reindex chucks to allow ordering
            int[] idx = {0};
            sortedBlocks.stream().forEachOrdered(block -> block.setIdx(idx[0]++));

            // persist in db
            // Store in class for ease of demonstration

        }
    }

The InterlacedStrategy class is the strategy implementation that selects the next two numbers from the relevant blocks as per the details above.

public class InterlacedStrategy implements Strategy<Integer> {

@Override
public List<Integer> execute(Config config) {

    // Should retrieve the blocks from the database
    List<Block> sortedBlocks = Utils.sortedBlocks;

    // Select the first two blocks
    Block block1 = sortedBlocks.get(0);
    block1.setStatus(Block.Status.IN_USE);
    Block block2 = sortedBlocks.get(1);
    block2.setStatus(Block.Status.IN_USE);

    // Generate number from block1
    Integer previousNumber = block1.getPreviousNumber();
    Integer nextNumberBlock1;
    if (previousNumber == null) {
        nextNumberBlock1 = block1.getStartOfBlock();
    } else {
        nextNumberBlock1 = ++previousNumber;
    }
    block1.setPreviousNumber(nextNumberBlock1);

    // Generate number from block2
    previousNumber = block2.getPreviousNumber();
    Integer nextNumberBlock2;
    if (previousNumber == null) {
        nextNumberBlock2 = block2.getEndOfBlock();
    } else {
        nextNumberBlock2 = --previousNumber;
    }
    block2.setPreviousNumber(nextNumberBlock2);

    if (nextNumberBlock1.equals(block1.getEndOfBlock())) {
        block1.setStatus(Block.Status.DEAD);

        List<Block> tempArray = sortedBlocks.stream()
                .filter(block -> block.getStatus() == Block.Status.LIVE)
                .collect(Collectors.toList());

        if (!tempArray.isEmpty()) {
            block1 = tempArray.get(0);
            block1.setStatus(Block.Status.IN_USE);
        } else {
            return new ArrayList<>();
        }
    }

    if (nextNumberBlock2.equals(block2.getStartOfBlock())) {
        block2.setStatus(Block.Status.DEAD);

        List<Block> tempArray = sortedBlocks.stream()
                .filter(block -> block.getStatus() == Block.Status.LIVE)
                .collect(Collectors.toList());

        if (!tempArray.isEmpty()) {
            block2 = tempArray.get(0);
            block2.setStatus(Block.Status.IN_USE);
        } else {
            return new ArrayList<>();
        }
    }

    // Should persist the blocks to the database

    List<Integer> numbers = new ArrayList<>();
    numbers.add(nextNumberBlock1);
    numbers.add(nextNumberBlock2);
    return numbers;
}

}

I would appreciate a review of two things:

1. The use of the enum strategy pattern, but more importantly the generation of the interlaced blocks of numbers
2. Selection of those numbers

I have no doubt that there is a far more elegant way to generate the interlaced blocks.

Here is a GitHub repository of all the code complete with passing unit tests.

Answer 1

From top to bottom:

---

startOfBlockand endOfBlock never change during the lifetime of a block. I find it confusing that a block knows about it's own index, but if you need to persist it into a database that's probably the least bad option.

In either case: There's no valid block, that has any of idx, startOfBlock, endOfBlock, previousNumber. Map that into the class properties themselves and just keep ints:

private int idx;
private final int startOfBlock;
private final int endOfBlock;
private int previousNumber;

It seems to me that it should be possible to also have idx final, but that would require adjustments as to how you create the blocks.

---

public static List<Block> sortedBlocks = new ArrayList<>();

I shall assume that this is not your actual implementation, because otherwise I'd have to throw a fit of rage and shouting to reduce global state.

Moving swiftly on to:

List<Block> listOddBlocks = blocks
    .stream()
    .filter(block -> block.getIdx() % 2 == 1)
    // ...
List<Block> listEvenBlocks = blocks
    .stream()
    .filter(block -> block.getIdx() % 2 == 0) 
    // ...

This can be significantly shorter (and one iteration over blocks less) as:

Map<Boolean, List<Block>> partitionBlocks = blocks.stream()
    .sorted(Comparator.comparing(Block::getIdx))
    .collect(Collectors.partitioningBy(block -> block.getIdx() % 2 == 1));
sortedBlocks.addAll(partitionBlocks.get(true));
sortedBlocks.addAll(partitionBlocks.get(false));

---

Other than that I have the huge grievance with your code, that you're apparently eagerly computing a sequence of non-repeating numbers. If you want to make it look random, why not simply use a RNG?