We use cookies to ensure you have the best browsing experience on our website. Please read our cookie policy for more information about how we use cookies.
Using loops without brackets when it using only 1 line is totally readable. If you push too much in one line, its not ok. Guys shoud have used ternary operator for real 1 line here. There is basically 2 lines
Agree, I think its much better readability if we use curly brackets only if a loop or if() statement has more than 2 lines. I hate when people use brakets liket this thou:
//we could stop at sqrt(n)//This is my Java solution. I think this may be better.classCalculatorimplementsAdvancedArithmetic{publicintdivisorSum(intn){intsum=0;for(inti=1;i<=(int)Math.sqrt(n);i++){if(n%i==0){sum=sum+i+n/i;}}if((int)Math.sqrt(n)*(int)Math.sqrt(n)==n){sum-=Math.sqrt(n);}returnsum;}}
class Calculator implements AdvancedArithmetic {
public int divisorSum(int n) {
int sum=n;
int i=1;
while(i<=n/2){
if(n%i==0)
sum+=i;
i++;
}
return sum;
}
}
Performance geek here. Nice job. You might want to try improving the runtime by only iterating up to the square root of n instead of n like this to improve your runtime from O(n) to O(n^(1/2)).
Iterating up to sqrt(n) makes our runtime improve from O(n) to O(n^(1/2)). When we are checking a number for divisors, we want to find 2 numbers that multiply into a 3rd number, like this:
a*b=c
In my code, the variables are named differently, so we want
i*n/i=n
Every time we find 1 divisor i, we also add the other divisor n/i to our sum. Since we are adding both sets of divisors, we can get all of them iterating just up to the sqrt(n). If we kept iterating, we would get duplicates.
As an example, if i = 2, n/i = 5, n = 10, we would get 2 * 5 = 10, so we grab both divisors: 2 and 5. If we kept iterating past the square root, we would eventually reach i = 5, n/i = 2, and n = 10, and we would again grab divisors: 5 and 2. We don't want that, so we stop iterating when we reach the square root.
Did your code compile? Because they told us to not use the public keyword for class definition, only for method definition...because were defining multiple classe in one file.☝
Day 19: Interfaces
You are viewing a single comment's thread. Return to all comments →
C# - Just kept it simple, I'm sure the performance geeks are screaming.
I like this one, it's clean and easy to follow.
Also, since it's a one liner
for loop, you should be able to getaway with removing the curley braces.I mean, since we're shortening code...
Then you start losing readability. But does look interesting. Just one of those things C# has that Java doesn't.
i have got only so:
@rtmrdnk I'm getting correct result by your code but not by below code in Java8, can you tell why?
for(int i=1;i<=n;sum+=(n%i++==0?i:0));
I cracked up at work. You won the internet today.
You could, but not considered a good programming practice from the point of view of readability and avoiding strange bugs
Using loops without brackets when it using only 1 line is totally readable. If you push too much in one line, its not ok. Guys shoud have used ternary operator for real 1 line here. There is basically 2 lines
This should be like this for 100% readability.
Agree, I think its much better readability if we use curly brackets only if a loop or if() statement has more than 2 lines. I hate when people use brakets liket this thou:
Better readable like this:
we need to check till halfway only (n/2).
at least you could stop checking at
n/2and addnat the endYeah it's a very easy optimization, here's how mine looked:
That's clever
u diid't declare half?
I din't understand why you are doing the
part. Can you explain?
If n is square number, you have to minus sqrt n. Just math.
You can optimize that one step to the following... to avoid correction for doubble addition, when input number is an exact square.
sum = sum+i; should be sum = sum-i;
Simple code is the best :)
ss u r correct @gowthambala1200 know this very well
here is my simple and optimized code:
class Calculator implements AdvancedArithmetic { public int divisorSum(int n) { int sum=n; int i=1; while(i<=n/2){ if(n%i==0) sum+=i; i++; } return sum; } }
You missed the bounds checks.
Nobody really checks those...
Performance geek here. Nice job. You might want to try improving the runtime by only iterating up to the square root of n instead of n like this to improve your runtime from O(n) to O(n^(1/2)).
Hope this helps.
HackerRank solutions.
any explainations for iterating up to sqrt(n)? and could you please explain sum += i + n/i; part as well?
Thank you.
Iterating up to sqrt(n) makes our runtime improve from O(n) to O(n^(1/2)). When we are checking a number for divisors, we want to find 2 numbers that multiply into a 3rd number, like this:
In my code, the variables are named differently, so we want
Every time we find 1 divisor i, we also add the other divisor n/i to our sum. Since we are adding both sets of divisors, we can get all of them iterating just up to the sqrt(n). If we kept iterating, we would get duplicates.
As an example, if i = 2, n/i = 5, n = 10, we would get 2 * 5 = 10, so we grab both divisors: 2 and 5. If we kept iterating past the square root, we would eventually reach i = 5, n/i = 2, and n = 10, and we would again grab divisors: 5 and 2. We don't want that, so we stop iterating when we reach the square root.
HackerRank solutions.
My realization of what you suggested:
Nice. This line looks a little weird to me:
Any integer % 1 is always 0. So it seems that line is just checking if the square root of n is an integer or not.
HackerRank solutions.
yes, that's what I do to avoid checking
inside the loop.
I am on your side! My code is similar to yours - one for loop, simple and clean.
Better use ternary for 1 liner if.
nice yll its really helped me
Did your code compile? Because they told us to not use the public keyword for class definition, only for method definition...because were defining multiple classe in one file.☝
At least only do the loop up to sqrt of n lol
no need to take loop till n, if u take int sum=n; then condition (till half loop) : i<=n/2;