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.

Challenge link

Discussions related to implementation:
About code structure
Optimization

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

Would appreciate feedback not only about algorithm, but about comments in code.

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()

there is no need for additional comments.

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

if nothing of it possible >>> add more comments

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.