5
\$\begingroup\$

The main function tries to create a urllib connection, and if it fails/succeeds the label changes accordingly. However, I'm looking for ways to improve this code.

Some elements I'm already aware of:

  • The use of place really isn't ideal, I'm aware
  • Importing Urllib twice, but if I didn't do it like this the except: wasnt working correctly
  • Use of global

import urllib
import urllib.request as url
from tkinter import *

master = Tk()
master.geometry("280x36")
master.resizable(False, False)
master.config(bg="#4b4b4b")
master.overrideredirect(True)

master.wm_geometry("-0-40")
master.wm_attributes("-topmost", 1)

connectStatus = Label(bg="#e67e22", fg="#fff", width=29, height=1, text="Pending", font="Bahnscrift 10")
connectStatus.place(x=10,y=8)

closeButton = Button(width=2, bg="#706f6f", fg="#fff", borderwidth=0, text="x", command=master.destroy)
closeButton.place(x=253, y=8)

def displayCheck():
    colorFlash = "#38f789" if callbackAttempt == 1 else "#ff6656"
    connectStatus.config(bg=colorFlash)
    master.after(10, runRefresh)

def runRefresh():
    attemptConnect()
    master.after(2000, displayCheck)

def attemptConnect():
    global callbackAttempt

    try:
        callbackAttempt = 1
        callback = url.urlopen("https://stackoverflow.com", timeout=1)
        connectStatus.config(text="Connection Established", font="Bahnscrift 10", bg="#2ecc71")
        callback.close()

    except urllib.error.URLError:
        connectStatus.config(text="No Connection", bg="#e74c3c", font="Bahnscrift 10")
        callbackAttempt = 0

master.after(5, runRefresh)
master.mainloop()
\$\endgroup\$
1
  • \$\begingroup\$ Edit: Changed displayCheck() function to use ternary instead. colorFlash = "#38f789" if callbackAttempt == 1 else "#ff6656" \$\endgroup\$
    – Chris
    Feb 20 '18 at 1:25
1
\$\begingroup\$

It's a neat idea, I like it! There is however a couple different things that could be done differently, while retaining the original idea.

Syntax

Have a look at PEP-8, it's a universal style guide for Python, it will give you an idea of how to style your code. As an example: functions, methods and variables should be snake_case, not mixedCase or CamelCase. While the use of the latter won't raise a syntax error, using PEP-8 is preferred.

Try to avoid wildcard imports (importing contents of entire libraries), as it will pollute your namespace. In the point below I suggest a library that's not part of the standard library, which should be separated from standard library imports, by one line.

import tkinter as tk

import requests

attemptConnect function

I would consider switching to a higher level web request solution, the requests library. It comes with all the features we need out of the box and additionally it provides an extremely handy Session object. Basically, it will use the underlying TCP connection making the subsequent requests faster. We will send a HEAD request, so the content won't be requested.

At first I thought the global variable is a counter of some sort, but a closer look made me realise it acts as a boolean variable. Why not make it one? This change will also have an effect in your ternary: "#38f789" if request_successful else "#ff6656".

I've taken the liberty to start changing function and variable names to follow PEP-8.

session = requests.Session()
...
def verify_url_status():
    global request_successful
    try:
        session.head('https://stackoverflow.com/')
        request_successful = True
        status_label.config(text="Connection Established", bg="#2ecc71", font="Bahnscrift 10")
    except requests.exceptions.ConnectionError:
        status_label.config(text="No Connection", bg="#e74c3c", font="Bahnscrift 10")
        request_successful = False

Classes

I have never worked much with tkinter, but this problem begs for the use of a class, encapsulating your code with it would make it more modular and would solve the need for a global variable. I will refer you to this tutorial, as it will be more comprehensive than what I know. If you haven't touched object oriented programming in Python yet, I'd recommend reading up on it as well. Below is the basic idea.

import tkinter as tk

import requests


class URLStatus:

    def __init__(self):
        self.session = requests.Session()
        self.request_successful = None
        self.master = tk.Tk()
        ...
        self.status_label = tk.Label(bg="#e67e22", fg="#fff", width=29, height=1, text="Pending", font="Bahnscrift 10")
        self.status_label.place(x=10,y=8)

    def verify_url_status(self):
        try:
            self.session.head('https://stackoverflow.com/')
            self.request_successful = True
            self.status_label.config(text="Connection Established", bg="#2ecc71", font="Bahnscrift 10")
        except requests.exceptions.ConnectionError:
            self.status_label.config(text="No Connection", bg="#e74c3c", font="Bahnscrift 10")
            self.request_successful = False
\$\endgroup\$
3
  • \$\begingroup\$ Hello. Thanks for all the feedback, I appreciate the time you took to answer this :) I'll be sure to work on this when I get a chance with your changes in mind. I'm mainly using URLLIB as it comes with python by default. \$\endgroup\$
    – Chris
    Feb 21 '18 at 23:14
  • 1
    \$\begingroup\$ I generally like to use from module import foo, bar, baz for all my imports (even though I have to do it manually), but I'm not sure how common that is. \$\endgroup\$ Mar 24 '18 at 1:58
  • \$\begingroup\$ You can do that either. @SolomonUcko \$\endgroup\$
    – Lukasz
    Mar 24 '18 at 1:59

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.