1
\$\begingroup\$

I am writing a Python class which checks if a given number is divisible by numbers 2-11. I am not using the __init__ method which in my opinion is not necessary in this case.

Questions:

  • How can I convert a given number in to abs(x) without using the __init__ method in every method at once?
  • How can I check if a number is divisible by 7 without using a recursive solution?

class Divisibility:
    """
    Program checks if given number is divisible
    by any number from 2 to 11
    """

    # Divisibe by 2 if last digit is divisible by 2
    def divisible_by_2(self, number):
        number = abs(number)
        if (number % 10) % 2 == 0:
            return True
        else:
            return False

    # Divisible by 3 if sum of its digits is divisible by 3 
    def divisible_by_3(self, number):
        number = abs(number)
        n = number

        s = 0
        while n:
            s, n = s + n % 10, n // 10

        if s % 3 == 0:
            return True
        else:
            return False

    # Divisible by 4 if last two digits is divisible by 4
    def divisible_by_4(self, number):
        number = abs(number)
        if (number % 100) % 4 == 0:
            return True
        else:
            return False

    # Divisible by 5 if last digit is either 5 or 0
    def divisible_by_5(self, number):
        number = abs(number)
        if number % 10 == 0 or number % 10 == 5:
            return True
        else:
            return False

    # Divisible by 6 if number is divisible by 2 and 3
    def divisible_by_6(self, number):
        number = abs(number)
        if self.divisible_by_2(number) and self.divisible_by_3(number):
            return True
        else:
            return False


    def divisible_by_7(self, number):
        number = abs(number)
        if number % 7 == 0:
            return True
        else:
            return False

    # Divisible by 8 if last three digits is divisible by 8
    def divisible_by_8(self, number):
        number = abs(number)
        if (number % 1000) % 8 == 0:
            return True
        else:
            return False

    # Divisible by 9 if the sum of its digits is divisible by 9
    def divisible_by_9(self, number):
        number = abs(number)
        s = 0
        while number:
            s, number = s + number % 10, number // 10

        if s % 3 == 0:
            return True
        else:
            return False

    # Divisible by 10 if last digit is 0
    def divisible_by_10(self, number):
        number = abs(number)
        if number % 10 == 0:
            return True
        else:
            return False

    # Divisible by 11 if the difference between
    # the sum of numbers at even possitions and odd
    # possitions is divisible by 11
    def divisible_by_11(self, number):
        number = abs(number)
        n = number
        nn = number // 10
        total = 0

        while n:
            total += n % 10
            n //= 100

        while nn:
            total -= nn % 10
            nn //= 100

        if abs(total) % 11 == 0:
            return True
        else:
            return False

if __name__ == '__main__':

    D = Divisibility()
    print(D.divisible_by_2(-6))
    print(D.divisible_by_3(6))
    print(D.divisible_by_4(6))
    print(D.divisible_by_5(60))
    print(D.divisible_by_6(6))
    print(D.divisible_by_7(616))
    print(D.divisible_by_8(82453))
    print(D.divisible_by_9(9512244))
    print(D.divisible_by_10(-571288441))
    print(D.divisible_by_11(121))
\$\endgroup\$
5
\$\begingroup\$

There is a programmer inside joke that says "no self respecting ethical programmer would ever consent to write a bomb Baghdad function. They would write a bomb city function to which Baghdad could be passed as a parameter."

This is the biggest issue with the code here: the divisibility functions could all be rolled together into one with the number you want to check passed as a parameter. That also addresses the concern you rightly raise about the abs thing.

Note in particular that the way computers store numbers is different from how we write them, so although it is easier and quicker for us to have shortcuts like check the last digit for evenness, it is actually slower for the computer to find what the last digit is than just check the whole thing against two! Therefore, it is quite ok to use the same style for all of them as you use for seven. And that means there is nothing in the way with merging them. (Technically, there are slightly faster options for 2, 4, and 8 but having the code be the same in each case is more important than using them.)

Lastly, I like the fact that you have some test cases written down. I suggest that there is room for doing that a bit more systematically. Ensure that you cover some yeses, some noes, and any special cases that could cause trouble. (negatives, zero, the number itself, etc) for each function you write separate code for. For example, I think the 9 function could do with a bit more of a prod. Try throwing 6 at it for example. ;) (Not to labour the point, but simplifying the testing needed to be sure everything is right is a key reason to use common functions. )

\$\endgroup\$
5
\$\begingroup\$

You do this quite alot

if s % 3 == 0:
    return True
else:
    return False

Instead you could return directly

return s % 3 == 0
\$\endgroup\$
3
\$\begingroup\$

You are a creating an instance of the Divisibility class, and never storing any data inside it. This makes one question why you are even using a class at all; you could instead just use functions divisible_by_2(...) and so on.

Still, there are times when a class can have methods associated with it that use no internal data. These methods are referred to as "static" methods. You would decorate the method with @staticmethod and remove the self parameter. Eg)

(With tests and return statements simplified, as pointed out by @Ludisposed.)

class Divisibility:
    """..."""

    # Divisibe by 2 if last digit is divisible by 2
    @staticmethod
    def divisible_by_2(number):
        return (abs(number) % 10) % 2 == 0

You actually are using the self parameter in the divisible_by_6 method, but only to refer to other static methods of this class. There is something between regular methods and static methods, called "class methods", which instead of receiving the object they are operating on as the first parameter, they receive the class they are operating on as the first parameter. This allows them to call other class methods and static methods:

    # Divisible by 6 if number is divisible by 2 and 3
    @classmethod
    def divisible_by_6(cls, number):
        return cls.divisible_by_2(number) and cls.divisible_by_3(number)

Both divisible_by_3 and divisible_by_9 methods require computing the sum of the digits of the number. This common code should be factored out into its own (private) method, where it can be written clearer, with better variable names, and removing the unnecessary tuple packing/unpacking

    @staticmethod
    def _sum_of_digits(number):
       number = abs(number)
       digit_sum = 0

       while number:
           digit_sum = digit_sum + number % 10
           number = number // 10

       return digit_sum

Then it can be used by both functions, without duplication of the code.

    # Divisible by 3 if sum of its digits is divisible by 3 
    @classmethod
    def divisible_by_3(cls, number):
        return cls._sum_of_digits(number) % 3 == 0

    # Divisible by 9 if the sum of its digits is divisible by 9
    @classmethod
    def divisible_by_9(cls, number):
        return cls._sum_of_digits(number) % 3 == 0    # WTF?

At this point, it might become clear that your logic for your divisible_by_9 method had a typo / bug.

Also, one should avoid doing the same calculations multiple times. In divisible_by_5 you calculate the last digit twice: once to test it against 0, and again to test it against 5. Instead, you should save the calculated last digit, and test that value against both 0 and 5, avoiding the second modulo-10 operation:

    # Divisible by 5 if last digit is either 5 or 0
    @staticmethod
    def divisible_by_5(number):
        last_digit = abs(number) % 10
        return  last_digit == 0  or  last_digit == 5
\$\endgroup\$
  • 1
    \$\begingroup\$ or use the in-operator: number % 10 in [0, 5] \$\endgroup\$ – Daniel Sep 23 '18 at 18:05
1
\$\begingroup\$

If you are going for speed, fastest is simplest:

def divisible(number, divisor):
    return number % divisor == 0
\$\endgroup\$
0
\$\begingroup\$

Better implement to check if number is divisible to 2,4,8 just need to check whether the last 1,2,3 bits separately to be all 0

For example for 8

def divisible_by_8(self, number):
    number = abs(number)
    return number&0b111 == 0

and for divisible by 3 you can use binary regex pattern (0|1(01*0)*1)*, haha not what you want I just think funny, I am thinking maybe there is also a magic pattern for 7

\$\endgroup\$

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.