I did the Divisible Sum Pairs problem on HackerRank
input: n is length of array ar, k is an integer value.
a pair is define only if ar[i]+ar[j] is dividable by k where i < j.
Problem: find and return pairs in array.
def divisibleSumPairs(n, k, ar):
i=0 #pointer 1
j=1 #pointer 2 is faster than p1
count =0 #pointer 3 will only increment 1 once j = n-1 (second last element)
pairs=0
while count < n-1:
if (ar[i]+ar[j])%k==0:
pairs+=1
j+=1
count+=1
if count==n-1:
i+=1
j=i+1
count =i
print(pairs)
Code visualized
step 1:
i j
[a][b][c][e][f]
compare (i,j)
i j
[a][b][c][e][f]
compare (i,j)
i j
[a][b][c][e][f]
compare (i,j)
i j
[a][b][c][e][f]
compare (i,j)
i+=1
j=i+1
step2
i j
[a][b][c][e][f]
compare (i,j)
.
.
.
.
step n
i j
[a][b][c][e][f]
compare (i,j)
finish
I have a couple of questions:
- Does this algorithm have a name?
- Is there other way (maybe better, faster)?
- Is there some code I could have done better/ differently?
#some test data n=6 , k=3 ar =[1, 3, 2, 6, 1, 2]
-
\$\begingroup\$ Welcome to CodeReview. Did you check the discussions on HackerRank for this challenge? There is a decent solution posted there with explanation. You can find it here \$\endgroup\$impopularGuy– impopularGuy2020年05月13日 10:13:32 +00:00Commented May 13, 2020 at 10:13
-
\$\begingroup\$ Thanks @impopularGuy. I did. However, the top voted code, written by usinha02, is stated to be O(n) time complexity. I don't understand how this can be when the code is using nested for loops? Shouldn't it then be O(n^2) time complexity? I guess it has something to do with the way the array is split into buckets of k i%k where i= 0,1,..k divide and conquer -ish? \$\endgroup\$MisterIvan– MisterIvan2020年05月13日 10:43:35 +00:00Commented May 13, 2020 at 10:43
-
\$\begingroup\$ I agree that the one I linked is not O(n) but is rather O(nk). However have a look at an explanation and the python implementation \$\endgroup\$impopularGuy– impopularGuy2020年05月13日 10:52:01 +00:00Commented May 13, 2020 at 10:52
1 Answer 1
Your code is not Pythonic as it does not adhere to the standard Python style guide.
divisibleSumPairs
should be snake casedivisible_sum_pairs
.- You should have one space both sides of most operators.
i=0
i+=1
,ar[i]+ar[j]
are all harder to read than they need to be. - Most of your variable names don't describe what they contain.
- You
print
inside a function that shouldreturn
.
def divisible_sum_pairs(length, divisor, items):
i = 0 #pointer 1
j = 1 #pointer 2 is faster than p1
count = 0 #pointer 3 will only increment 1 once j = n-1 (second last element)
pairs = 0
while count < length - 1:
if (items[i] + items[j]) % divisor == 0:
pairs += 1
j += 1
count += 1
if count == length - 1:
i += 1
j = i + 1
count = i
return pairs
You don't need pointer 3 as
count = j + 1
:j += 1 count += 1
j = i + 1 count = i
We can just replace
count
withj - 1
.We can simplify all of the places where
count
was being used. This is as you have- 1
s on both sides of the operators.Rather than
while j < length
it would be clearer if you instead usedwhile i < length - 1
. This comes in two parts:- It is confusing to see
while j < length
withif j == length
in the body. - You're not actually bound by
j
you're bound byi
.
- It is confusing to see
Rather than using a
while
loop we can see that there's an opportunity to use two for loops to make things easier to read.Note: These for loops have the exact same time complexity as your
while
.
def divisible_sum_pairs(length, divisor, items):
pairs = 0
for i in range(length - 1):
for j in range(i + 1, length):
if (items[i] + items[j]) % divisor == 0:
pairs += 1
return pairs
We can simplify the code by using using a comprehension.
For example we can extract getting the combinations of items.
combinations = ( (items[i], items[j]) for i in range(length - 1) for j in range(i + 1, length) ) for a, b in combinations: if (a + b) % divisor == 0:
We can instead
sum
with a comprehension that generates numbers.pairs = sum( 1 for a, b in combinations if (a + b) % divisor == 0 )
We can exploit the fact that bools are integers and move the if's expression as the comprehension's expression.
- We can use
itertools.combinations
to remove the need to manually get the combinations.
import itertools
def divisible_sum_pairs(_, divisor, items):
return sum(
(a + b) % divisor == 0
for a, b in itertools.combinations(items, 2)
)
Every change has been a 1 to 1 replacement. The time complexity is still \$O(\binom{n}{2})\$ (\$O(n^2)\$) and memory complexity is still \$O(1)\$.
However now readability is much better.
-
\$\begingroup\$ nice answer. Only remark I have is that instead of the
for i in range(length - 1)
in the intermediate solution, I would introduceenumerate
here. \$\endgroup\$Maarten Fabré– Maarten Fabré2020年05月13日 14:55:52 +00:00Commented May 13, 2020 at 14:55 -
\$\begingroup\$ @MaartenFabré I had thought of using
enumerate
but getting all bar the last value with it is not really that clean. \$\endgroup\$2020年05月13日 15:06:09 +00:00Commented May 13, 2020 at 15:06