2
\$\begingroup\$

I am a Math tutor and I found the site https://www.yksityisopetus.fi/ where one can search pupils. I made a Python parser to find pupils that want Math teaching with at least 25 €/hour. Are there any improvements for this code?

import requests
import tkinter as tk

link = "https://www.yksityisopetus.fi/opiskelijat/matematiikka/min-25-per-tunti/uusin-ensin"
f = requests.get(link)
lista = f.text.split('\n')
ilmot = []
for i in range(len(lista)):
    if "<a href=\"/" in lista[i] and "<a href=\"/opettajat" not in lista[i] and "<a href=\"/contact" not in lista[i]:
        if lista[i] not in ilmot:
            ilmot.append(lista[i])
parsittu = []
for i in range(len(ilmot)):
    ilmo = "https://www.yksityisopetus.fi"+ilmot[i].split('"')[1]
    f = requests.get(link)
    f = f.text.split(">")
    for j in range(len(f)):
        if f[j] == '\n<br /':
            parsittu.append(f[j+1])
s = ""
parsilmot = []
for i in range(len(parsittu)):
    if parsittu[i].endswith('</div'):
        parsilmot.append(parsittu[i][0:len(parsittu[i])-5])

p = list(set(parsilmot))
quote = ""
for i in p:
    quote += i+"\n"

root = tk.Tk()
S = tk.Scrollbar(root)
T = tk.Text(root, height=4, width=100)
S.pack(side=tk.RIGHT, fill=tk.Y)
T.pack(side=tk.LEFT, fill=tk.Y)
S.config(command=T.yview)
T.config(yscrollcommand=S.set)
T.insert(tk.END, quote)
tk.mainloop()
\$\endgroup\$
  • 2
    \$\begingroup\$ I think you can drastically reduce the size of this code. But I don't want to misread anything. Can you provide a short example input and output and explain a little bit of what's going on. \$\endgroup\$ – Peilonrayz Jun 22 at 20:54
  • 2
    \$\begingroup\$ Rather than using text searching, it would be much cleaner to use BeautifulSoup to parse the HTML. \$\endgroup\$ – Ken Y-N Jun 23 at 0:11
5
\$\begingroup\$

Move your code into functions

Functions are easier to test, and make it easier to modify your code over time.

For example, I think the first block requests information from a server and creates a list of links. I think the second block processes that list of links and generates some other sort of list. I think the third block iterates over the list from the second block and generates yet another list.

If these blocks were in functions, then the main logic would be self explanatory. For example, your code could look like:

...
ilmot = get_students(link)
parsittu = some_other_function(ilmot)
parsilmot = another_function(parsittu)
quote = create_quote(parsilmot)
...

If those functions all had reasonable names, the code would be very easy to understand, and it would be easy to test each function in isolation.

Not only that, but you can more easily move all of that code into a function so that you can implement a "refresh" feature into the UI so that you don't have to restart the script every time you want to update the data.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

English

For better or worse, English is the lingua franca of programming. It is completely fine for user-facing content to be localised to Finnish, but the code should not be (e.g. lista, parsittu), for a handful of reasons. Python itself uses English words in its syntax, so doing otherwise in the rest of the code is inconsistent.

Parsing HTML

It is very risky to do this manually as you are:

if "<a href=\"/" in lista[i]

it's fragile and prone to failure. Use BeautifulSoup instead.

Checking requests calls

After this:

f = requests.get(link)

call

f.raise_for_status()

Otherwise, failures will be non-obvious.

| improve this answer | |
\$\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.