Since we're dealing with an int array, we could also use .clone() to make a new array with the same content, but it may be faster to use copyOf but it may be faster to use copyOf.
NoSuchElementException is a runtime exception. Those unchecked exceptions (contrary to checked ones that must be declared in the signature of the method) should not generally be in the signature should not generally be in the signature. Prefer to document it in the Javadoc than add it in the signature of the method.
Since we're dealing with an int array, we could also use .clone() to make a new array with the same content, but it may be faster to use copyOf.
NoSuchElementException is a runtime exception. Those unchecked exceptions (contrary to checked ones that must be declared in the signature of the method) should not generally be in the signature. Prefer to document it in the Javadoc than add it in the signature of the method.
Since we're dealing with an int array, we could also use .clone() to make a new array with the same content, but it may be faster to use copyOf.
NoSuchElementException is a runtime exception. Those unchecked exceptions (contrary to checked ones that must be declared in the signature of the method) should not generally be in the signature. Prefer to document it in the Javadoc than add it in the signature of the method.
private static final int[] START_COUNTS = { /* A */ 9, /* B */ 2, 2, 4, 12, 2, 3, 2, 9, 1, 1, 4, /* M */ 2, /* N */ 6, /* O */ 8, 2, 1, 6, 4, 6, 4, 2, 2, 1, 2, /* Z */ 1, /* junk */ -1, /* junk */ -1, /* junk */ -1, /* junk */ -1, /* _ */ 2 };
What are the junk values for in the START_COUNTS array?
They do not represent a valid Scrabble tile, which are defined as letters A through Z with the blank tile _. It may not be clear for all readers that those junk values correspond to the characters that are between Z and _ in the ASCII table. Consider documenting exactly what they are for and why they are initialized with the magic value of -1.
private int[] counts = Arrays.copyOf(START_COUNTS, START_COUNTS.length);
Since we're dealing with an int array, we could also use .clone() to make a new array with the same content, but it may be faster to use copyOf.
public void remove(char c) throws NoSuchElementException {
NoSuchElementException is a runtime exception. Those unchecked exceptions (contrary to checked ones that must be declared in the signature of the method) should not generally be in the signature. Prefer to document it in the Javadoc than add it in the signature of the method.
This pattern is also used in the JDK itself, it is possible to take as example the Stream API introduced in Java 8: limit(maxSize) is documented to throw an IllegalArgumentException if the given maxSize is negative, and this exception is not present in the method signature.
try { return this.n; } finally { this.n = // ...; }
The usage of the try-finally construct here could be surprising. It is in fact not used to deal with resources that ought to be closed, but to make sure that the current value of n is returned, while n is updated to a next value that is the result of some calculation.
I would argue that the try-finally construct should be restricted to dealing with resources, instead of using cleverly its logic like this (because it may not be clear to everyone, and could even really be obscure).
It would be a lot clearer to just keep the old value in a local variable:
public Integer next() {
int currN = this.n;
this.n = Arrays.stream(START_COUNTS).filter(x -> x < n).max().orElse(-1);
return currN;
}
The proposed solution is using a int[] to store the current count for the each Scrabble tiles. It is then using an iterator to iterate over all the possible count of values and get the letters with that count. One of the possible drawback is that this ties implicitely the code into the sorting logic that is used in the problem. If tomorrow, the problem is changed and asks for output in reverse order, then an iterator logic will need to be changed, instead of a sorting logic.
In fact, I would not use an iterator at all. Although this was probably done on purpose with your solution, I would use a Map to keep the current count of character to the character. It wouldn't introduce a real memory overheard since we're not dealing with a lot of values to store, and it would have the great advantage that this data structure could then be sorted properly, independently of the way the count is being built.
Consider the following instead:
Map<Integer, List<String>> result =
IntStream.range(0, counts.length)
.boxed()
.collect(Collectors.groupingBy(
i -> counts[i],
() -> new TreeMap<>(Collections.reverseOrder()),
Collectors.mapping(i -> String.valueOf((char)(i + 'A')), Collectors.toList())
));
result.entrySet()
.stream()
.map(e -> e.getKey() + ": " + String.join(", ", e.getValue()))
.forEach(System.out::println);
The first part of the code goes through the counts array and create a Map<Integer, List<String>> with the number of letter left and a String representing all the characters at that count. It uses a TreeMap as backing map to ensure the keys are correctly ordered in descending order.
Then, it is possible to post-process that Map and simply print its content.