Naming is hard!
There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.[1]
Have you tried reading your code out loud? The naming isn't very readable.
Looking at the specification I would expect the code to read something like:
Construct largest number from digits
count digits
create largest number from digit counts
return result
Translating that to Java we get:
public static long constructLargestNumberFrom(int digits) {
int[] digitCounts = getDigitCounts(digits);
long largestNumber = constructLargestNumberFromDigitCounts(digitCounts);
return largestNumber;
}
Where the two functions are the first and second half of your original code.
What about negative numbers?
You either need to disallow them:
if(digits < 0) {
throw new IllegalArgumentException("digits must be greater than or equal to zero.");
}
Or handle them:
private static int[] getDigitCounts(int digits) {
int[] digitCounts = new int[10];
while (digits != 0) {
int digit = Math.abs(digits % 10);
digitCounts[digit]++;
digits /= 10;
}
return digitCounts;
}
Notice the Math.abs
call in the digit extraction. Just constructing the largest number from a negative number and ignoring the sign doesn't seem right though. It should be the least negative, i.e. the smallest number possible from the digits. With everything combined we get:
public static long constructLargestNumberFrom(int digits) {
return constructLargestNumberFrom(digits, 10);
}
public static long constructLargestNumberFrom(int digits, int base) {
if(base < 1) {
throw new IllegalArgumentException("Base must be positive");
}
int[] digitCounts = getDigitCounts(digits, base);
long largestNumber = digits < 0 ?
constructSmallestNumberFromDigitCounts(digitCounts) * -1:
constructLargestNumberFromDigitCounts(digitCounts);
return largestNumber;
}
private static int[] getDigitCounts(int digits, int base) {
assert base > 0 : "base must be greater than zero";
int[] digitCounts = new int[base];
while (digits != 0) {
int digit = Math.abs(digits % base);
digitCounts[digit]++;
digits /= base;
}
return digitCounts;
}
private static long constructLargestNumberFromDigitCounts(int[] digitCounts) {
assert digitCounts.length > 0 : "digitCounts must contains atleast one digit count.";
assert Arrays.stream(digitCounts).allMatch(count -> count >= 0) : "Counts cannot be negative";
int base = digitCounts.length;
long largestNumber = 0;
for (int digit = base - 1; digit >= 0; digit--) {
for (int count = 0; count < digitCounts[digit]; count++) {
largestNumber = largestNumber * base + digit;
}
}
return largestNumber;
}
private static long constructSmallestNumberFromDigitCounts(int[] digitCounts) {
assert digitCounts.length > 0 : "digitCounts must contains atleast one digit count.";
assert Arrays.stream(digitCounts).allMatch(count -> count >= 0) : "Counts cannot be negative";
int base = digitCounts.length;
long smallestNumber = 0;
for (int digit = 0; digit < base; digit++) {
for (int count = 0; count < digitCounts[digit]; count++) {
smallestNumber = smallestNumber * base + digit;
}
}
return smallestNumber;
}
I also removed all the 10
's scattered around the code and replaced them with base
. I find this much more readable and now you can also do the same for other bases.
If you have any preconditions in you methods you should always clearly show them in your code.
- In public methods you should throw exceptions.
- In private methods you should do assertions.
Then anyone using your public methods will get a nice error and when you debug you can turn on assertions (with the vmflag -ea) and get nice errors when you don't follow your own guidelines. When assertions aren't turned on they're identical to empty statements and have close to zero performance implications.