5
\$\begingroup\$

This is probably horribly sloppy, but here it is! This is a section from a large (imo) GUI program I am making. The below code is what triggers when a specific button is pressed.

 def calc_platenumber(self):
    customer_firstname = str(self.customer2_entry.get())
    customer_lastname = str(self.customer3_entry.get())

    infile = open('license_numbers.txt', 'r')

    license_numbers = infile.readlines()

    infile.close()

    self.platenmb = random.choice(license_numbers)

    self.platenumber.set(self.platenmb)

    license_numbers.remove(self.platenmb)

    outfile = open('license_numbers.txt', 'w')

    for item in license_numbers:
        outfile.write(item)

    outfile.close()

    infile = open('used_license_numbers.txt', 'r')
    used_license_numbers = infile.readlines()
    infile.close()

    used_license_numbers.append(self.platenmb)
    used_license_numbers.append(customer_firstname)
    used_license_numbers.append(customer_lastname)

    outfile = open('used_license_numbers.txt', 'w')

    for item in used_license_numbers:
        outfile.write(item)

    outfile.close()

So, I have a GUI that asks for the first name and last name of the person. There is a generate button below that. The generate button will take a random license number from a large list of available numbers, remove it from that list, and then add it to a list of "used" license numbers. That is the function you see here. When it adds it to this second list, I want it to also take their name and add it as well. I have made it do that.

Problem is, whenever it adds it, it looks sloppy. No spaces or anything. I would rather it add on like...

Tim Bob C565213

And then whenever the next person uses the program, the file is updated to say...

Tim Bob C565213
Jim Bean 1234F324

...And so on. Currently it just looks sloppy, and even if sloppy won't matter (this is just a school project), it is going to bother me.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Clean code is good just for the habit of writing clean code. \$\endgroup\$ – James Khoury Dec 3 '13 at 23:15
5
\$\begingroup\$

You could do a better job describing the task; the task description should be a docstring. I would also rename the function to select_platenumber(), since it's not really calculating anything.

If you're using plain text files as a database, watch out for race conditions. Unless you implement some kind of locking mechanism, you could easily end up with corrupted files.

As @JamesKhoury says, you should only have to open each file once, using a with block. Use the appropriate mode: you only need to append to the output file. For the input file, you can truncate and overwrite it.

For efficiency, I prefer to select a random array index rather than a random array element. Selecting a random element is more readable, but to remove that element involves a linear search through the list.

I think it would be safer to write the used_license_numbers.txt entry before overwriting license_numbers.txt, in case of an I/O error. (You could have inconsistent data, but at least you would be less likely to lose data.)

Having similarly variables self.platenmb and self.platenumber is confusing.

def select_platenumber(self):
    """Selects and removes a random line from license_numbers.txt as the
       license plate, and appends a line to used_license_numbers.txt
       recording the license plate and customer name.
    """
    firstname = str(self.customer2_entry.get())
    lastname = str(self.customer3_entry.get())

    with open('license_numbers.txt', 'r+') as in_file, \
         open('used_license_numbers.txt', 'a') as out_file:
        plates = in_file.readlines()
        self.platenmb = plates.pop(random.randrange(len(plates))).rstrip()
        self.platenumber.set(self.platenmb)

        out_file.write('%-19s %s %s\n' % (plate, firstname, lastname))

        in_file.truncate(0)
        in_file.writelines(plates)
\$\endgroup\$
1
  • \$\begingroup\$ If you don't want to use the format strings, you can also use the string functions ljust, rjust and center to pad you coluns in the output docs.python.org/2/library/string.html \$\endgroup\$ – theodox Dec 5 '13 at 1:16
4
\$\begingroup\$

You're opening the files more than once like:

infile = open('used_license_numbers.txt', 'r')
used_license_numbers = infile.readlines()
infile.close()

Instead write it like this:

with open('used_license_numbers.txt', 'r') as infile:
    used_license_numbers = infile.readlines()

(python 2.6 +)

Also you're opening the same file twice, once for reading and once for writing. Could you open it for both reading and writing?

open('used_license_numbers.txt', 'r+')
\$\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.