4
\$\begingroup\$

I have a function that returns postal codes between the ones given. It works but I feel like I made this over-complicated and there's easy way to make it more simple but I have no idea how.

Inputs are two strings that represents postal codes of my country. They are in the format 'XX-XXX' where X is [0-9]. The task is to return postal codes between them.

Example:

Input: '80-998' '81-003'

Output: '80-998' '80-999' '81-000' '81-001' '81-002' '81-003'


def codes():
    x = '79-900'
    y = '80-155'

    x1 = int(x.split('-')[0])
    x2 = int(x.split('-')[1])
    y1 = int(y.split('-')[0])
    y2 = int(y.split('-')[1])

    while x1 <= y1:
        if x1 == y1:
           while x2 <= y2:
                print ('{}-{:03d}'.format(x1,x2))
                x2 += 1
        else:
            while x2 < 1000:
                print ('{}-{:03d}'.format(x1,x2))
                x2 += 1
        x1 += 1
        x2 = 0
\$\endgroup\$
1

2 Answers 2

5
\$\begingroup\$

Reusability

As it stand your code compute zip codes between '79-900' and '80-155'. In case you want to change these bounds, you have to modify the code itself. Which means you can even call this function twice with different bounds within the same program.

You should instead make x and y parameters of the function:

def codes(x, y):
    # Rest of the code

And then you can call it like:

codes('79-900', '80-155')
codes('80-998', '81-003')

You can also benefit from using more descriptive variable names here; like begin and end or start and stop. Same applies for the function name, something like zip_codes_range is more meaningful.

Range

The name zip_codes_range is not chosen arbitrarily. It is because it expand over the name of the range function. And this is because, in the end, zip codes can be viewed as integers and the range function is meant to generate "integers between the ones that are given" (note how this is close to your title).

So the idea is to just parse the zip codes into integers and then feed them to range so it can generate intermediate values:

def int_from_zip(zip_code):
    parts = zip_code.split('-')
    value = ''.join(parts)
    return int(value)


def zip_codes_range(start, stop, step=1):  # Mimic `range` signature
    start = int_from_zip(start)
    stop = int_from_zip(stop)
    for zip_code in range(start, stop+1, step):
        begin = zip_code // 1000
        end = zip_code % 1000
        print('{:02}-{:03}'.format(begin, end))

You could also use begin, end = divmod(zip_code, 1000) instead of the 2 lines process.

Print vs return

To make your function more useful, you should separate concerns. This function should only serves to compute the needed zip codes and return them somehow; the printing of the values being done in another part of the code.

This allow any program using this function to use the value as they intend to, and not necessarily print them.

The first thing you can do is return a list of all the requested values:

def int_from_zip(zip_code):
    return int(''.join(zip_code.split('-')))


def zip_codes_range(start, stop, step=1):
    start = int_from_zip(start)
    stop = int_from_zip(stop)
    return [
        '{:02}-{:03}'.format(*divmod(zip_code, 1000))
        for zip_code in range(start, stop+1, step)
    ]


if __name__ == '__main__':
    for code in zip_codes_range('79-900', '80-155'):
        print(code)

But you could consider using generators, the same way range changed from returning a list in Python 2 to being a generator in Python 3:

def zip_codes_range(start, stop, step=1):
    start = int_from_zip(start)
    stop = int_from_zip(stop)
    for zip_code in range(start, stop+1, step):
        yield '{:02}-{:03}'.format(*divmod(zip_code, 1000))
\$\endgroup\$
1
\$\begingroup\$

This is a short version handling some invalid inputs.

def gen_codes(start, end):
    """
    Return a generator for a list of postal codes (XX-XXX) from 'start' to 'end'

    >>> list(gen_codes('80-998', '81-003'))
    ['80-998', '80-999', '81-000', '81-001', '81-002', '81-003']
    >>> list(gen_codes('81-003', '80-998'))
    []
    >>> list(gen_codes('', ''))
    []
    """
    try:
        sn = int(start.replace('-', ''))
        en = int(end.replace('-', ''))
        return ('%02d-%03d' % (d // 1000, d % 1000) for d in range(sn, en+1))
    except ValueError:
        return iter([])

As mentioned by Mathias one should pass the start and stop postal codes as arguments to the function in order to make it reusable.

Concerning the implementation it is a good idea to return a generator instead of a list as it is easy to iterate over them, apply filters and only use part of the results.

In addition one can think about error handling, specially how the function should behave in case of invalid input. In the above code I decided that an invalid range, or a postal code that does not follow the format XX-XXX returns an empty list.

I personally like to have some doctests as a form of documentation and simple test in the function description.

\$\endgroup\$
1
  • \$\begingroup\$ I think you should just let the ValueError propagate instead of making it return an empty iterator. The empty iterator would lead to mysterious bugs. \$\endgroup\$ Sep 26, 2016 at 14:33

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.