HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

Does this tkinter-based web browser widget use a well implemented class?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisclassimplementedtkinterbrowserwellbaseddoeswebuse

Problem

I have fully functional code in class format, and as far as I am aware from my previous question, it conforms to PEP 8 and the code logic and implementation is suitably pythonic. My main concerns here is whether or not I have put self. in front of the correct variables, and some ambiguity surrounding my @staticmethod.

  • Should I leave it as it is?



  • Should I change the logic so it is not static?



  • Should it be in the class at all?



```
# This imports some necessary libraries.
import webbrowser
import tempfile
import urllib.request
from tkinter import *

class Browser:
"""This creates a relay that allows a user to directly view data sent from a web server."""
def __init__(self, master):
"""Sets up a browsing session."""
# Explicit global declarations are used to allow certain variable to be used in all methods.
global e1

# Here we create some temporary settings that allow us to create a client that ignores proxy settings.
self.proxy_handler = urllib.request.ProxyHandler(proxies=None)
self.opener = urllib.request.build_opener(self.proxy_handler)

# This sets up components for the GUI.
Label(master, text='Full Path').grid(row=0)
e1 = Entry(master)
e1.grid(row=0, column=1)
Button(master, text='Go', command=self.browse).grid(row=0, column=2)

# This binds the return key to self.browse as an alternative to clicking the button.
root.bind('', self.browse)

@staticmethod
def parsed(data):
"""Cleans up the data so the file can be easily processed by the browser."""
# This removes removes all python-added special characters such as b'' and '\\n' to create understandable HTML.
initial = str(data)[2:-1]
lines = initial.split('\\n')
return lines

def navigate(self, query):
"""Gets raw data from the queried server, ready to be processed."""
# This gets the opener to query our request, and submit

Solution

Class Scoping

So your comment afore your global declaration of e1 mentions how you wish for e1 to be accessible in all methods. If you want a variable to be accessible in all methods of a class, the conventional way is to assign it as a property of self:

class Foo():
     def __init__(self, bar):
         self.boz = bar * 3

     def baz(self):
         # boz is now accessible here
         print(self.boz)


Note that boz is not a global; if I create two Foo() objects, I'll end up with 2 different boz variables:

>>> f1 = Foo(3)
>>> f1.boz
9
>>> f2 = Foo(2)
>>> f2.boz
6
>>> f1.boz
9 # Still


Static Methods

To answer your (most conceptual) questions about the static method:

  • Potentially



  • The function takes some data, and then performs an operation on it. It does not rely on any other state to decide what operation to perform, so it is a perfect candidate for being static.



  • No, it does not need to be static; a static method is (conceptually) a function in all but name, so you could write it as a function. The main reasons to have something be a static variable is usually organizational or conceptual, so if you think that applies here, keep it as a staticmethod. Otherwise, consider making it a bog standard function.



There is no definite rule for when to write a function and when to write a static method; it's something you learn over time. The two questions I ask myself when deciding to write a static method are "Could I write this as a function?" and "Is this code very tightly linked (conceptually) to the class?". If the answer to both of those questions is yes, then I might write it as a static method.

In this case, I think a function might be more useful.

General bugs

Just some little things I noticed:

  • Your parsed static method says that it removes cruft from a string, but it never checks to see if the characters it expects to be there actually exist. Personally, I would use a regexp to implement this function, but regardless, it might be wise to check that the characters you are deleting are the ones you expect to be there.



  • In __init__ you do root.bind(...), where you could do master.bind(...), which might be better, as it would avoid using a global variable.



  • I'd recommend using a new style class by replacing class Browser: with class Browser(object):. The benefits of the syntax difference are subtle, but noticeable (and it helps with forward compatibility iirc).



  • It might be a better idea to define a main function, and then call that instead of just doing loads of stuff in the global scope. You could also add a guard case of the form if __name__ == "__main__": main() to allow you to import the script into a REPL without running the code.



  • Some of your variable names kinda suck. Try and find more descriptive names than e1!

Code Snippets

class Foo():
     def __init__(self, bar):
         self.boz = bar * 3

     def baz(self):
         # boz is now accessible here
         print(self.boz)
>>> f1 = Foo(3)
>>> f1.boz
9
>>> f2 = Foo(2)
>>> f2.boz
6
>>> f1.boz
9 # Still

Context

StackExchange Code Review Q#42177, answer score: 10

Revisions (0)

No revisions yet.