2
\$\begingroup\$

It can be seen that the number, 125874, and its double, 251748, contain exactly the same digits, but in a different order.

Find the smallest positive integer, x, such that 2x, 3x, 4x, 5x, and 6x, contain the same digits.

The following code works on SpiderMonkey 1.8.5. But didn't work on my nodejs.
It takes about two seconds, and gives the correct answer.

My two major concerns are my style. And my choice in functions.
Personally I think str_sorted is horrible, int -> str -> array -> str.

// Like Python's range.
function range(start, stop, step){
    if (typeof(stop)==='undefined'){
        stop = start;
        start = 0;
    }
    if (typeof(step)==='undefined') step = 1;
    if (stop === null){
        while (true){
            yield start;
            start += step;
        }
    }else{
        for (number = start; number < stop; number += step){
            yield number;
        }
    }
}


function str_sorted(num){
    num = num.toString();
    arr = Array();
    for (index in num){
        arr.push(num[index]);
    }
    return arr.sort().join('');
}


function permuted(num){
    tmp = str_sorted(num);
    return tmp == str_sorted(num * 2) &&
           tmp == str_sorted(num * 3) &&
           tmp == str_sorted(num * 4) &&
           tmp == str_sorted(num * 5) &&
           tmp == str_sorted(num * 6);
}


function find(iterable, fn){
    for (num in iterable){
        if (fn(num)){
            return num;
        }
    }
}


print(find(range(1, null), permuted));

The way that I run this code is via the command line.

$ js p52.js
142857
$

This runs the the SpiderMonkey 1.8.5 interpreter. I got it from the Arch repository at some point.

\$\endgroup\$
5
  • \$\begingroup\$ yield? In a normal function? Did you mean function* range(...? \$\endgroup\$ – Madara's Ghost Jul 12 '15 at 12:35
  • \$\begingroup\$ You forgot to mention that this is, partially, ES6. Also, where did that print came from? \$\endgroup\$ – Ismael Miguel Jul 12 '15 at 12:39
  • 3
    \$\begingroup\$ @JoeWallis yield is an ECMAScript 2015 (previously known as ECMAScript 6 or ES6 for short) feature. It only works together with generators (function*). Which is why I find it surprising that this code even works to begin with :o \$\endgroup\$ – Madara's Ghost Jul 12 '15 at 12:42
  • \$\begingroup\$ I ran this code on the latest IOJS REPL on my machine and got this. The IOJS REPL natively supports ES6 and generators, so I'm curious as to how this code is working (because it shouldn't be). \$\endgroup\$ – Dan Jul 12 '15 at 13:53
  • \$\begingroup\$ I've checked the MDN page for the version of SpiderMonkey that @Joe is using and it shows that it is outdated. The fact this code works appears to be a bug in SpiderMonkey. That said, OP says it works for him in SpiderMonkey 1.8.5. I'm unsure as to where this topic falls as a result. SM 1.8.5 uses ES5, in which the yield keyword should be a syntax error because it's a reserved word. \$\endgroup\$ – Dan Jul 12 '15 at 13:56
2
\$\begingroup\$

Overall, your code looks pretty decent. The indentation is spot-on, the variable names make sense, the code is clear and easy to read. But there are some big issues with your code.

  1. The first thing that popped in my eyes was this:

    if (typeof(stop)==='undefined'){
    

    Please, don't use typeof to check if it is defined or not. You have a very interesting object called arguments.

    Try this instead:

    if (arguments.length < 3){
    
  2. A few lines below, you have this:

    if (stop === null){
    

    What if I pass undefined or false?

    You need to predict those, since they are valid values and, somewhat, in context. And yes, it is possible to pass undefined to a function, in one of these ways:

    range(1, undefined, 3); //simply pass undefined
    range.call({}, 1, undefined, 3); //mostly the same
    range.apply({}, [1,,3]); //it may happen!
    //... more similar variants ...
    

    You need to take care of those.

  3. This is one of a few examples.

    You have the following function:

    function str_sorted(num){
        num = num.toString();
        arr = Array();
        for (index in num){
            arr.push(num[index]);
        }
        return arr.sort().join('');
    }
    

    Let's focus on the following bit:

    arr = Array();
    

    There are 2 wrong things here:

    • This is a local variable without var
    • You are using the Array constructor as a function
      You either use [] or use new Array()

    The fix is simple:

    function str_sorted(num){
        var arr = [];
        num = num.toString();
        for (index in num){
            arr.push(num[index]);
        }
        return arr.sort().join('');
    }
    

    But you still have something there that's bothering me:

        num = num.toString();
        for (index in num){
            arr.push(num[index]);
        }
    

    Why are you converting it to a string and then iterating it? Is it to sort the numbers in the string? If that's the case, here's the whole function:

    function str_sorted(num){
        return num.toString().split('').sort().join('');
    }
    

    Using string.split('') will split a string by each character, making an array of numbers.

  4. Let's check your permuted() function.

    It also suffer from a tiny flaw:

    function permuted(num){
        tmp = str_sorted(num);
        return tmp == str_sorted(num * 2) &&
               tmp == str_sorted(num * 3) &&
               tmp == str_sorted(num * 4) &&
               tmp == str_sorted(num * 5) &&
               tmp == str_sorted(num * 6);
    }
    

    I have no idea what you are trying to do, but here is the mistake:

    tmp = str_sorted(num);
    

    There, you forgot a var.

    Change it to this:

    var tmp = str_sorted(num);
    
  5. Lets take a look at the find function.

    This one isn't too bad:

    function find(iterable, fn){
        for (num in iterable){
            if (fn(num)){
                return num;
            }
        }
    }
    

    Just more of the same:

    for (num in iterable){
    

    You forgot a var there.

    Change it to this:

    for (var num in iterable){
    
  6. I'm not sure if this is part of the code to be reviewed or not.

    But let's analyze the last line:

    print(find(range(1, null), permuted));
    

    Please, don't use print. It isn't even a standard! It's just supported in some consoles (like Chrome). Use console.log() instead.

    For now, I will leave the review as is. If I find any more issues, in the future, I will check on them.

\$\endgroup\$
4
  • \$\begingroup\$ The arguments.length check won't protect you from range(undefined, undefined, undefined) ... \$\endgroup\$ – janos Jul 12 '15 at 14:45
  • \$\begingroup\$ @janos The 'original' check won't protect from range(null, null, null) or range(0, 0, 0) or even range(false, false, false). \$\endgroup\$ – Ismael Miguel Jul 12 '15 at 15:47
  • \$\begingroup\$ sure, the original wasn't perfect to begin with. I'm not saying your suggestion is wrong, but it has a negative side to it too, which deserves mention \$\endgroup\$ – janos Jul 12 '15 at 15:52
  • \$\begingroup\$ @janos I've already partially mentioned this problem on point 2. \$\endgroup\$ – Ismael Miguel Jul 12 '15 at 15:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.