Skip to main content
deleted 2 characters in body
Source Link
Peilonrayz
  • 44.5k
  • 7
  • 80
  • 158

Changes I'd make, from the top down:

  • Use string with enumerate to make the dictionary. Rather than manually making one.
  • letters is a constant, so use LETTERS.
  • Make a new function to change the layout of an IBAN and change it to a number.
  • Rename chech_validation_chars_iban to the name generate_iban_check_digits as it's not a check, it's a transform.
  • Change the format to use strings, {:0>2}, and remove the if.
  • Change validate_iban to check if it's a valid IBAN, and so return a boolean.
  • Use and to reduce the amount of ifs.

This resulted in:

import string
LETTERS = {ord(d): str(i) for i, d in enumerate(string.digits + string.ascii_uppercase)}


def _number_iban(iban):
    return (iban[4:] + iban[:4]).translate(LETTERS)


def generate_iban_check_digits(iban):
    number_iban = _number_iban(iban[:2] + '00' + iban[4:])
    return '{:0>2}'.format(98 - (int(iban_numberednumber_iban) % 97))


def valid_iban(iban):
    return int(_number_iban(iban)) % 97 == 1


if __name__ == '__main__':
    my_iban = 'RO13RZBR0000060007134800'
    if generate_iban_check_digits(my_iban) == my_iban[2:4] and valid_iban(my_iban):
        print('IBAN ok!\n')
    else:
        print('IBAN not ok!\n')

There is one glairing problem with this, the specification states that it allows both upper and lowercase input:

Replace the letters in the string with digits, expanding the string as necessary, such that A or a = 10, B or b = 11, and Z or z = 35. Each alphabetic character is therefore replaced by 2 digits

However we only allow uppercase. To amend this I'd change the letters dict to include the lowercase letters. But makes the code a little harder to read, and not as clean. And so the cleanest way would probably be to use itertools.chain, with the method we're using now.

_LETTERS = chain(enumerate(string.digits + string.ascii_uppercase),
                 enumerate(string.ascii_lowercase, 10))
LETTERS = {ord(d): str(i) for i, d in _LETTERS}

Changes I'd make, from the top down:

  • Use string with enumerate to make the dictionary. Rather than manually making one.
  • letters is a constant, so use LETTERS.
  • Make a new function to change the layout of an IBAN and change it to a number.
  • Rename chech_validation_chars_iban to the name generate_iban_check_digits as it's not a check, it's a transform.
  • Change the format to use strings, {:0>2}, and remove the if.
  • Change validate_iban to check if it's a valid IBAN, and so return a boolean.
  • Use and to reduce the amount of ifs.

This resulted in:

import string
LETTERS = {ord(d): str(i) for i, d in enumerate(string.digits + string.ascii_uppercase)}


def _number_iban(iban):
    return (iban[4:] + iban[:4]).translate(LETTERS)


def generate_iban_check_digits(iban):
    number_iban = _number_iban(iban[:2] + '00' + iban[4:])
    return '{:0>2}'.format(98 - (int(iban_numbered) % 97))


def valid_iban(iban):
    return int(_number_iban(iban)) % 97 == 1


if __name__ == '__main__':
    my_iban = 'RO13RZBR0000060007134800'
    if generate_iban_check_digits(my_iban) == my_iban[2:4] and valid_iban(my_iban):
        print('IBAN ok!\n')
    else:
        print('IBAN not ok!\n')

There is one glairing problem with this, the specification states that it allows both upper and lowercase input:

Replace the letters in the string with digits, expanding the string as necessary, such that A or a = 10, B or b = 11, and Z or z = 35. Each alphabetic character is therefore replaced by 2 digits

However we only allow uppercase. To amend this I'd change the letters dict to include the lowercase letters. But makes the code a little harder to read, and not as clean. And so the cleanest way would probably be to use itertools.chain, with the method we're using now.

_LETTERS = chain(enumerate(string.digits + string.ascii_uppercase),
                 enumerate(string.ascii_lowercase, 10))
LETTERS = {ord(d): str(i) for i, d in _LETTERS}

Changes I'd make, from the top down:

  • Use string with enumerate to make the dictionary. Rather than manually making one.
  • letters is a constant, so use LETTERS.
  • Make a new function to change the layout of an IBAN and change it to a number.
  • Rename chech_validation_chars_iban to the name generate_iban_check_digits as it's not a check, it's a transform.
  • Change the format to use strings, {:0>2}, and remove the if.
  • Change validate_iban to check if it's a valid IBAN, and so return a boolean.
  • Use and to reduce the amount of ifs.

This resulted in:

import string
LETTERS = {ord(d): str(i) for i, d in enumerate(string.digits + string.ascii_uppercase)}


def _number_iban(iban):
    return (iban[4:] + iban[:4]).translate(LETTERS)


def generate_iban_check_digits(iban):
    number_iban = _number_iban(iban[:2] + '00' + iban[4:])
    return '{:0>2}'.format(98 - (int(number_iban) % 97))


def valid_iban(iban):
    return int(_number_iban(iban)) % 97 == 1


if __name__ == '__main__':
    my_iban = 'RO13RZBR0000060007134800'
    if generate_iban_check_digits(my_iban) == my_iban[2:4] and valid_iban(my_iban):
        print('IBAN ok!\n')
    else:
        print('IBAN not ok!\n')

There is one glairing problem with this, the specification states that it allows both upper and lowercase input:

Replace the letters in the string with digits, expanding the string as necessary, such that A or a = 10, B or b = 11, and Z or z = 35. Each alphabetic character is therefore replaced by 2 digits

However we only allow uppercase. To amend this I'd change the letters dict to include the lowercase letters. But makes the code a little harder to read, and not as clean. And so the cleanest way would probably be to use itertools.chain, with the method we're using now.

_LETTERS = chain(enumerate(string.digits + string.ascii_uppercase),
                 enumerate(string.ascii_lowercase, 10))
LETTERS = {ord(d): str(i) for i, d in _LETTERS}
Source Link
Peilonrayz
  • 44.5k
  • 7
  • 80
  • 158

Changes I'd make, from the top down:

  • Use string with enumerate to make the dictionary. Rather than manually making one.
  • letters is a constant, so use LETTERS.
  • Make a new function to change the layout of an IBAN and change it to a number.
  • Rename chech_validation_chars_iban to the name generate_iban_check_digits as it's not a check, it's a transform.
  • Change the format to use strings, {:0>2}, and remove the if.
  • Change validate_iban to check if it's a valid IBAN, and so return a boolean.
  • Use and to reduce the amount of ifs.

This resulted in:

import string
LETTERS = {ord(d): str(i) for i, d in enumerate(string.digits + string.ascii_uppercase)}


def _number_iban(iban):
    return (iban[4:] + iban[:4]).translate(LETTERS)


def generate_iban_check_digits(iban):
    number_iban = _number_iban(iban[:2] + '00' + iban[4:])
    return '{:0>2}'.format(98 - (int(iban_numbered) % 97))


def valid_iban(iban):
    return int(_number_iban(iban)) % 97 == 1


if __name__ == '__main__':
    my_iban = 'RO13RZBR0000060007134800'
    if generate_iban_check_digits(my_iban) == my_iban[2:4] and valid_iban(my_iban):
        print('IBAN ok!\n')
    else:
        print('IBAN not ok!\n')

There is one glairing problem with this, the specification states that it allows both upper and lowercase input:

Replace the letters in the string with digits, expanding the string as necessary, such that A or a = 10, B or b = 11, and Z or z = 35. Each alphabetic character is therefore replaced by 2 digits

However we only allow uppercase. To amend this I'd change the letters dict to include the lowercase letters. But makes the code a little harder to read, and not as clean. And so the cleanest way would probably be to use itertools.chain, with the method we're using now.

_LETTERS = chain(enumerate(string.digits + string.ascii_uppercase),
                 enumerate(string.ascii_lowercase, 10))
LETTERS = {ord(d): str(i) for i, d in _LETTERS}