A simple parcel checker Python script

Question

My current student accommodation uses a manual paper and pen system for checking if students have mail so I made this simple Python script to help mitigate the problem of manually searching through spreadsheets just to see if you have mail or not, which I have been asked to demo soon. It's quite basic but I figured I should probably get it code reviewed since it may get real world usage.

GitHub

import json
import os

if not os.path.isfile('parcels.json'):
    open('parcels.json', 'w').write(json.dumps({}))
parcels = json.loads(open('parcels.json').read())

def add(student_id, new_letters, new_parcels):
    if student_id in parcels:
        parcels[student_id][0] += new_letters
        parcels[student_id][1] += new_parcels
    else:
        parcels[student_id] = [new_letters, new_parcels]

def collect(student_id):
    if student_id in parcels:
        parcel_info = 'Letter: ' + str(parcels[student_id][0]) + ' Parcel: ' + str(parcels[student_id][1])
        del parcels[student_id]
        return parcel_info
    else:
        return 'Letter: 0 Parcel: 0'

print('1. Add\n2. Collect')
option = input('Option: ')
student_id = input('Student ID: ')

if len(student_id) != 7:
    print('Invalid input! (student ID must be 7 digits long)')
    raise SystemExit(1)
try:
    if int(student_id) < 0:
        print('Invalid input! (student ID must be a non-negative integer)')
        raise SystemExit(1)
except ValueError:
    print('Invalid input! (student ID must be an integer)')
    raise SystemExit(1)

if option == '1':
    try:
        new_letters = int(input('Letters: '))
        new_parcels = int(input('Parcels: '))
    except ValueError:
        print('Invalid input! (only integers allowed)')
        raise SystemExit(1)

    if new_letters < 0 or new_parcels < 0:
        print('Invalid input! (only non-negative integers allowed)')
        raise SystemExit(1)

    add(student_id, new_letters, new_parcels)
elif option == '2':
    print(collect(student_id))
else:
    print('Invalid option!')

open('parcels.json', 'w').write(json.dumps(parcels))

Answer 1

Code organization

Defining the add() and collect() functions is a good start, but it could be better, as there is still a lot of free-floating code outside those functions. Part of the problem is that prompting for the inputs to add() requires intimate knowledge of what add() does, so you might as well make add() responsible for reading its own input.

Prompting for the Student ID is common to options 1 and 2, but is not appropriate for any other invalid option. Therefore, the Student ID prompting code needs to be moved.

parcels.json is hard-coded four times. The loading code is at the top, far from the saving code at the bottom. I also find it odd that you feel compelled to write an empty dictionary to parcels.json if the file does not exist.

I'd use the following outline:

import json
import re

DATABASE_FILENAME = 'parcels.json'

def load():
    try:
        with open(DATABASE_FILENAME) as f:
            return json.loads(f.read())
    except IOError as e:
        return {}

def save(data):
    …

def input_student_id(prompt='Student ID: '):
    while True:
        student_id = input(prompt)
        if re.match(r'^\d[7]$', student_id):
            return student_id
        print('Invalid input! Student ID must be 7 digits')

def input_nonnegative_int(prompt):
    …

def add(data):
    student_id = input_student_id()
    new_letters = input_nonnegative_int('Letters: ')
    new_parcels = input_nonnegative_int('Parcels: ')
    record = data.get(student_id, [0, 0])
    data[student_id] = [record[0] + new_letters, record[1] + new_parcels]

def collect(data):
    student_id = input_student_id()
    record = data.get(student_id, [0, 0])
    print('Letter: {0} Parcel: {1}'.format(*record))
    del data[student_id]

def main():
    data = load()
    while True:
        option = input('1. Add\n2.Collect\nOption: ')
        if option == '1':
            add(data)
            break
        elif option == '2':
            collect(data)
            break
        else:
            print('Invalid option!')
    save(data)

if __name__ == '__main__':
    main()

Besides reorganizing all of the code into functions, I've also changed the input validation. It's more forgiving — retrying instead of aborting. Also, using a regular expression to validate the student ID simplifies the code while forbidding "000.000" as input (which your program would have accepted).

Data format

Using a two-element list to store the letter and parcel count is hard to read and a headache for future expansion. A dictionary with two keys would be a bit better.

I'm concerned about using a JSON file as a database. It's not a storage mechanism that is designed for concurrent access, so you might end up with dropped add/collect transactions or even a corrupted file. If you don't want to install a full SQL database server, then consider using SQLite as a storage mechanism — it takes care of the concurrency concerns for you.