Dynamic Official Receipt increment algorithm

Question

I made a function that would automatically generate official receipt for POS. it is able to accept format such as xxx, xxx-xxx, xx-xx.

But my code is really messy and horrible and it needs review. I would really appreciate for corrections, recommendations and critiques to improve this.

The flow would be:

1. Get the last official receipt number OR, if none use specified system default.

2. If there is a provided format, use it. and read the last number according to its format

3. Split it, convert it to int and increment it.

4. Check it doesn't exist yet.
5. Return the next official receipt number

I'm using Django/Python. If it needs explanation please comment and I will explain the best way I can.

try:
 user = request.user.id
 official_receipt = {}
 default = {}
 company = get_current_company(request)
 or_from_last_transaction = ""
 or_from_default_company = ""
 try:
 try:
 official_receipt = Pointofsale.objects.filter(company=company,is_or_manual=False).values('official_receipt').order_by('-official_receipt').first()
 except Exception as e:
 or_from_last_transaction = '10000'
 else:
 pass
 finally:
 or_from_last_transaction = official_receipt['official_receipt']
 try:
 POS_settings.objects.filter(company=company).exists()
 except POS_settings.DoesNotExist:
 pass
 else:
 or_from_default_company = '00000'
 finally:
 default_company = POS_settings.objects.filter(company=company).values().first()
 if default_company['is_or_per_company']:
 or_from_default_company = default_company['pos_official_receipt']
 else:
 or_from_default_company = None
 except Exception as e:
 official_receipt = '30000'
 else:
 pass
 finally:
 official_receipt = or_from_default_company if or_from_default_company else or_from_last_transaction
 stop = True
 duplicate_add = 0
 if any(c.isalpha() for c in official_receipt):
 return 'Incorrect OR Format'
 while stop:
 str_group = official_receipt.split("-")
 str_length = len(official_receipt)
 group_length = int(len(str_group))
 for index, group in reversed(list(enumerate(str_group))):
 limit = len(group)
 group = int(group)
 group += 1
 if len(str(group)) > limit:
 if index == 0:
 str_group[index] = group
 else:
 str_group[index] = 0
 else:
 str_group[index] = int(str_group[index]) + (1 + duplicate_add)
 str_group[index] = str(str_group[index]).zfill(limit)
 break
 next_official_receipt = ''
 for group in str_group:
 next_official_receipt += str(group)
 next_official_receipt += '-'
 next_official_receipt = next_official_receipt[0:len(next_official_receipt)-1]
 if Pointofsale.objects.filter(official_receipt=next_official_receipt,company=company).exists():
 stop = True
 duplicate_add += 1
 else:
 stop = False
 return next_official_receipt
except Exception as e:
 print(e)
 return e

Answer 1

A few notes on the use of exceptions:

• You're catching the non-specific Exception. That's bad, because it hides real programming errors. You should be specific what exception to catch (like except POS_settings.DoesNotExist:).

• There are too many try-except blocks overall. You may need to rethink what you're trying to do, and with the previous point in mind, what exceptions you're trying to catch. Django queries may often return an empty queryset instead of raising an exception, which may be better suited.

• The overarching try-except is really bad. Firstly, that catches anything that isn't caught in the inner block, and secondly, it makes it hard to read where it starts and ends. Try-except blocks should be short, concentrated around the code code (one or a few lines at most) that might raise the exception

• If you use else: pass in an try-except clause (or in an if-clause), it's cleaner to just leave off the else branch. There's no use for it. The same goes for finally: pass.

• You're catching the overarching exception, then printing it, then returning it. That has two problems:

  • when run inside a proper server, print very likely causes problems (from my experience using Apache). Use the Django logging provided.

  • there is absolutely nothing to gain by returning an exception. Re-raise it, or perform a cleanup action (and possibly still re-raise it after that). Return is for actual values, not exceptions.

• Binsoi: Thank you Evert, i'll update the code and post it again.