Performance; base size 5 takes about 200 milliseconds on my machine, a base of 6 takes about 40 seconds, and 7 is still running after half an hour.
I can solve size=9 in 18 seconds, but I tried a very different approach (starting from the top and trying to compute the rows below). For size=10, it's still running after half an hour.
I don't claim that starting from the top is better, maybe starting from the base could be made much more efficient when more promising candidates get tried first. Looking at the solution
1000
489 511
277 212 299
175 102 110 189
116 59 43 67 122
77 39 20 23 44 78
50 27 12 8 15 29 49
32 18 9 3 5 10 19 30
21 11 7 2 1 4 6 13 17
you can see that the smallest values are in the middle of the base. This makes sense as they contribute most to the result.
Readability; how difficult is it to read the code
Not exactly easy, but not really bad. There are quite a few strange names and strange things done. For example, nextTry
both modifies it's input and returns it. This is pretty unexpected.
###checkTriangle
Your javadoc should be formatted like here.
int checkTriangle(int[] aTriangleBase, int aLimit) {
int size = aTriangleBase.length;
boolean[] count = new boolean[aLimit];
It's only a count
in a rather stretched sense. I'd suggest present
or found
.
// check input for duplicates
for (int i : aTriangleBase) {
if (count[i])
return -1;
count[i] = true;
}
The base row is a special case which should need no special handling.
int[] firstRow = new int[size];
int[] secondRow = Arrays.copyOf(aTriangleBase, size);
boolean useFirst = true;
int a = 0;
It took me a while till I found that you're reusing the arrays. It may be a useful optimization, but you it doubled your code.
for(int i = 1; i < size; ++i) {
if(useFirst) {
for(int j = 0; j < size-i; ++j) {
a = secondRow[j] + secondRow[j + 1];
if (a >= aLimit || count[a])
return -1;
You know, always braces. You're optimizing by aborting early on a >= limit
, but this should actually never happen, except for the top element. If this happens below, then the root element will be even bigger and such a triangle is simply too bad and should have been avoided earlier.
count[a] = true;
firstRow[j] = a;
}
useFirst = false;
} else {
for(int j = 0; j < size-i; ++j) {
a = firstRow[j] + firstRow[j + 1];
if (a >= aLimit || count[a])
return -1;
count[a] = true;
secondRow[j] = a;
}
useFirst = true;
Instead of this code duplication, you could simply swap firstRow
and secondRow
.
}
}
// return final value, our result if no duplicates occur during the process
return a;
}
The fact that you need a comment here is a sign that the method does not do exactly one right thing. It does two things, which may be fine when optimizing heavily, but I doubt you need it.
Two methods like
boolean makesValidTriangle(int[] base)
and
int triangleSum(int base)
would be way clearer. Note that the latter can easily be implemented using the pascal triangle. Note also that with further optimization, you won't even need it as you can compute the sum incrementally.
... to be continued...