# ProjectEuler problem 4. Feedback request

BE AWARE: this is working solution. If you want to solve it on your own, consider not to read the code.

Discussions related to implementation:
Optimization

Thanks to all who answered my questions, feel like I learned much when solving relatively easy problem.

I am using comments like I saw in MIT problem sets plus some improvisation. Course “Introduction to programming using Python”. Such problem sets are on MIT open course ware, I am sure anybody who wants can find it(I am not sure if link to such resource is appropriate for this forum).

Are there any conventional rules in JS in terms of commenting I am not using and should use?

Implementation:

``````function isNumberPalindromic(num) {
/*function was originally implemented to work with strings
was optimized to only work with numbers:
was renamed from palindrome() to isNumberPalindromic()
part of code for formatting strings(regexp) was replaced
Expected input:
num: natural number
Expected output:
true if number is palindromic
false otherwise
*/

let formattedNum = ""+num;
if (formattedNum=='' || formattedNum.length == 1) {
return true;
}
else {
return formattedNum[0] == formattedNum[formattedNum.length - 1] &&
isNumberPalindromic(formattedNum.slice(1,-1));
}
}

/*Unit testing
testing environment: onecompiler.com
console.log(isNumberPalindromic(9009));//true
console.log(isNumberPalindromic(9116));//false
console.log(isNumberPalindromic(9345));//false
*/

function largestPalindromeProduct(n) {
/*
Expected input:
n: natural number > 0, number of digits
Expected output:
number, the largest palindromic product of 2 n-digit numbers
*/

let result = 0;
for (let i = Math.pow(10, (n - 1)); i < Math.pow(10, n); i++) {
// if i == 2 >>> start step i = 10; end step i < 100

for (let j = i; j < Math.pow(10, n); j++) {
// j = i >>> exploiting math pattern to reduce number of checks
// TODO: try to estimate big O notation for these nested loops for the sake of learning

let tempResult = i * j; /*TODO:
figure out what's best: use variable or end up with more multiplications*/

if (tempResult > result && isNumberPalindromic(tempResult)) {
result = tempResult;
}
}
}

return result;
}

//all tests passed at fCC
``````

I agree,such comment of mine looks like not necessary explanation. I’ve just seen code examples where every function have descriptions like that no matter what’s in actual function body. I thought it was considered best practise to have expected input and expected output described for every function.

So for now I will stick to this:

If I have

``````veryDescriptiveFunctionName()
``````

If I have

``````vagueRidiculousVerboseLongFunctionName
``````

I need:
try to rename it first, maybe it will be better

if it not helps:
maybe function itself too complex >>> try to decompose it, split it

I’m on the other end of the spectrum with comments, but that’s because I write libraries.

In that context, writing docstrings for each function is important because that becomes the automatically generated documentation.

I think it’s generally considered a best practice to use === instead of == unless you explicitly intend to use type coercion.

I think that you can avoid a small amount od unneeded computation by starting with larger numbers and breaking the inner after a palendrome is found in the inner loop.

1 Like

You mean if I am looking for the largest, I can switch end steps and start steps and use decrement as an iteration step?
I tried that, but on earlier stages of implementation. I tried it in earlier versions of code, it didn’t work. But earlier code was crappy in general. I will try it.

You are referring to this line?

``````// if i == 2 >>> start step i = 10; end step i < 100

``````

If so, I think I get it.

The logic for your two loops is connected, right? You would need to change both.

Yeah, that’s the case

Yep, now it’s worked. New version(loops only).

``````    for (let i = (Math.pow(10, n) - 1); i > Math.pow(10, n - 1); i--) {
// if i === 2 >>> start step i = 99; end step i > 10

for (let j = i; j > Math.pow(10, n - 1); j--) {
// j = i >>> exploiting math pattern to reduce number of checks
// TODO: try to estimate big O notation for these nested loops for the sake of learning

let tempResult = i * j; /*TODO:
figure out what's best: use variable or end up with more multiplications*/

if (tempResult > result && isNumberPalindromic(tempResult)) {
result = tempResult;
break;
}
}
}
``````

I believe earlier I messed up something with Math.Pow, inside parenthesis.

You use this expression a few times. I think some (most?) Javascript engines recompute this value on every loop iteration. Something like `const smallestNDigitNumber = Math.pow(10, n - 1)` could add a bit of explanation and prevent the recalculation. Its a minor cost, but my rule of thumb is to store any calculation that is used multiple times (and not store a value only used once);

Got it. Better declare variable once than do bunch of calculations.

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.