Code Review:
Scenario #1: x negative, y positive; abs(x) > abs(y)
Let's review your code with example x = -8 and y = 5. The code will go into this section:
def divide(x, y): # x = -8 , y = 5
if x < 0 or y < 0: #satisfies x < 0 as -8 < 0
return 0 - 1 - divide(0 - x, y) # -1 -divide(8,5)
This will send the call back to divide function with values 8,5. It will skip all if statements and go into the below given else statement with values 8, 5.
def divide(x, y):
else:
return divide_helper(x, y, 0)
Let's assume divide_helper function gives you the correct result, you will receive a positive (+) result 1 because 8 can be divided by 5 once (1)
Taking this result back to the called function, you will get:
def divide(x, y): # x = -8 , y = 5
if x < 0 or y < 0:
return 0 - 1 - divide(0 - x, y) # return -1 -1 as [result of divide(8,5) = 1]
You are now going to return -2 instead of -1 which is incorrect.
Fix#1:
if x < 0 or y < 0:
return -divide(-x, y) #remove (0 - 1)
However, it does not address the sign issue
Scenario #2: x positive, y negative; abs(x) > abs(y)
Let's look at the code again with another example of x = 8 and y = -5. This time the code will go into section:
def divide(x, y): # x = 8 , y = -5
if x < 0 or y < 0: #satisfies y < 0 as -5 < 0
return 0 - 1 - divide(0 - x, y) # -1 -divide(-8,-5)
This goes through the same process and ends up with the same fix we discussed above. Again, this does not address the sign issue
Now let's review the second function that calculates the quotient:
def divide_helper(x, y, temp):
if y >= x:
return x
else:
return divide_helper(x - y, y, temp + 1)
Let's consider the example of x = 5, y = 8. The code satisfies the if statement and returns x which is 5. That's incorrect. It should return 0 as 5 cannot be divided by 8. Therefore this function is incorrect for values where x < y.
Similarly, if x = 5 and y = 5, it will return 5. That is also incorrect. It should have returned 1 as x divides by y only once (1).
Now let's look at x = 8, y = 5. In this case, it goes into the else statement calling divide_helper function with x = 3, y = 5. (x = x-y = 8-5 = 3). This time it returns 3. This is again incorrect.
Looking at your code, I think you wanted to return temp instead of x.
Fix#2:
def divide_helper(x, y, temp):
if y > x: #changed to only check if y > x
#When y=x, you need to increment temp by 1
return temp
elif x == y: #check explicitly for x==y and increment temp by 1
return temp+1
else:
return divide_helper(x - y, y, temp + 1)
These two fixes should address your divide problem. However, this does not address the sign issue. If x is + and y is - or if x is - and y is +, it will still return + instead of -. The updated and fixed code is as shown below:
def divide(x, y):
if x == 0 or y == 0:
return 0
if x < 0 and y < 0:
return divide(-x, -y)
if x < 0 or y < 0:
return divide(-x, y)
else:
return divide_helper(x, y, 0)
def divide_helper(x, y, temp):
if y > x:
return temp
elif x == y:
return temp+1
else:
return divide_helper(x - y, y, temp + 1)
While the above code does solve the quotient problem (excluding the sign issue), the code is calling the function too many times. In some cases, the call could have been avoided. Below is an alternate approach to solve this. See if it helps.
Alternate Approach:
In the below approach, the code does not call the function too many times. It also addresses the sign problem.
Here's the full implementation of the code. There are a few scenarios to address. You don't need 2 functions to calculate the results. Instead you can use a loop to iterate and increment the counter. Also, it is important to keep tab of the sign for each of the input parameters. This will decide the resultant (quotient) sign. As you know (+ and +) = +; (+ and -) = -; (- and +) = -; and (- and -) = +.
We also know that if numerator is 0 or numerator is less than denominator, the answer will be 0. If the denominator is 0, I am forcing a result of 0 (instead of infinity). For all other cases, we need to compute the value by iterating through the loop.
The full code with comments for each line is as follows:
def div_check(x, y):
#if either x or y is 0, return 0
if 0 in (x,y): return 0
#set the sign to positive (+)
s = 1
#if x & y are both negative numbers, result sign will be positive (+)
if (x < 0) and (y < 0): s = 1
#If x is negative and y is positive or the other way around, result sign will be negative (-)
if ((x < 0) and (y > 0)) or ((y < 0) and (x > 0)): s = -1
#convert x and y to absolute value so we can compute division
x,y = abs(x), abs(y)
#if x is less than y, then result is 0
if x < y : return 0
#initialize result r to zero
r = 0
#iterate through the loop each time x is greater than or equal to y while incrementing counter r by 1
while x >= y:
x-=y
r+=1
#multiply the sign s to result r to give the final result
return s*r
print ('(2,5) ',div_check(2,5))
print ('(-2,-5) ',div_check(-2,-5))
print ('(-2,5) ',div_check(-2,5))
print ('(-5,2) ',div_check(-5,2))
print ('(5,-2) ',div_check(5,-2))
print ('(-5,-2) ',div_check(-5,-2))
print ('(0,5) ',div_check(0,5))
print ('(2,0) ',div_check(2,0))
print ('(13,3) ',div_check(13,3))
print ('(-25,5) ',div_check(-25,5))
print ('(-39,-13)',div_check(-39,-13))
print ('(24,-7) ',div_check(24,-7))
print ('(3,14) ',div_check(3,14))
print ('(14,3) ',div_check(14,3))
#This also works for floats. The quotient will be integer
print ('(14.5,3.5)',div_check(14.5,3.5))
print ('(25.6,4.2)',div_check(25.6,-4.2))
The output of this will be:
(2,5) 0
(-2,-5) 0
(-2,5) 0
(-5,2) -2
(5,-2) -2
(-5,-2) 2
(0,5) 0
(2,0) 0
(13,3) 4
(-25,5) -5
(-39,-13) 3
(24,-7) -3
(3,14) 0
(14,3) 4
(14.5,3.5) 4
(25.6,4.2) -6
