Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
var phi=function(n){var count=" "  //an empty string
for(var i =1;i<=n;i++){
if(for(var j=1;j<=i;j++){
i%j===0;n%j===0;count += j.str;})   //appended the j as string into count
{return count.length;}}
};

The Euler totient function phi(n) gives the number of numbers <= n which are relatively prime to n.

share|improve this question

closed as off-topic by Veedrac, Quill, 200_success Jun 14 at 8:19

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Veedrac, Quill, 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

6  
Whitespace. Whitespace is good. –  QPaysTaxes Jun 7 at 17:42
    
What programming challenge is this? –  200_success Jun 7 at 18:59
    
My JS interpreters are telling me this isn't syntactically valid - which is as it should be. A for in an if?? –  Veedrac Jun 7 at 20:45
    
I was just trying to implement the Euler totient function.It was nothing to do with any particular programming challenge.I was trying to understand algorithms & implement them on my own.Well,if you people can suggest me anything on this would be very helpful. –  Siddy Jun 8 at 10:54

1 Answer 1

up vote 6 down vote accepted

Well, first off, whitespace:

var phi=function(n){var count=" "  //an empty string
for(var i =1;i<=n;i++){
if(for(var j=1;j<=i;j++){
i%j===0;n%j===0;count += j.str;})   //appended the j as string into count
{return count.length;}}
};

has far to little. I reformatted and made this (put in a couple of semicolons, too):

var phi = function (n) {
    var count = " ";  //an empty string
    for (var i = 1; i <= n; i++) {
        if (for (var j = 1; j <= i; j++) {
            i % j === 0;
            n % j === 0;
            count += j.str; //appended the j as string into count
        }) {
            return count.length;
        }
    }
};

" " is not an empty string. "" is.

Now, why the heck do you have a for in an if? Try rewriting it without one. Array#all ought to help you with that. Once I get a chance I'll try my hand at it.

.str isn't needed. Just write count += j.

I can't comment on your algorithm because I can't tell what it is. This is why we write readable code. Don't use stupid hacks that make your program .03% faster that make you use stupid, unreadable tricks like, say, putting a for in an if. Prioritize readability until you absolutely have to do otherwise.

share|improve this answer
    
Thanks for your reviews.I'll definitely follow it. –  Siddy Jun 7 at 18:34

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