I'm new to Python. For learning purposes, I want to create desktop application by using QtWebKit and Flask. You can find the source code here
Main idea is:
- create a form including a QWebview widget
- create a Flask app
- run Flask app on QWebview
It works correctly but I want to know:
- How can I improve my code?
- is this good way to create desktop apps?
My code is:
main.py
from core.Core import *
app = Flask(__name__)
from routes import *
if __name__ == '__main__':
main(app)
core module: core/Core.py
import sys
from flask import Flask , render_template
from PyQt4.QtCore import *
from PyQt4.QtGui import *
from PyQt4.QtWebKit import *
from threading import Thread
class FlaskThread(QThread):
def __init__(self,app):
QThread.__init__(self)
self.app = app
def __del__(self):
self.wait()
def run(self):
self.app.run()
def main(app):
th = FlaskThread(app)
th.start()
#qt init
qtapp = QApplication(sys.argv)
qtapp.aboutToQuit.connect(lambda : th.terminate())
webview = QWebView()
webview.load(QUrl('http://localhost:5000'))
webview.show()
sys.exit(qtapp.exec_())
and then routes.py
from main import *
@app.route('/')
def index():
#return 'hi'
return render_template('index.html')
1 Answer 1
Your imports and module structure seems like an incredible mess. Yet your application only contains 3 files. Let's try and improve that.
Value your namespaces
import this
Preferably in an interactive session, will give you an insight of how you should approach problems in Python. In your case, it is the last line Namespaces are one honking great idea -- let's do more of those!
which is at issue.
The core idea is to control where symbols can be found without risk of overriding them (or doing so knowingly). As such, from <something> import *
is a bad practice and should either be:
import <something>
or
from <something> import <required_name1>, ..., <required_nameN>
This also means that you should removed unnecessary imports.
Avoid circular imports
I'm talking about having from main import *
in routes.py
and from routes import *
in main.py
.
One of the major issue with circular import is that one of them might bring an half-loaded module, (mainly) leading to NameError
s.
Why do you need to import routes
in the first place? Nothing is used from that in main.py
, so you can remove it.
Why do you need to import main
in the first place? To be able to use render_template
and the app
object? Well, you should import render_template
in routes.py
and create app
in there instead.
Don't over-split your code
Modules are good since they allow you to differentiate purposes for pieces of codes. But too much modules makes the code unmanageable.
At this stage of development you could still have all your code in a single file without loosing readability. But I imagine that you'll want to make your application grow and I understand your need to split it into different modules.
You made a good job identifying purposes:
- a module to manage your webapp (
routes.py
); - a module to manage your GUI (in case you'll want to get something better than a
QWebView
in the future).
But I think you made a mistake making your GUI module "secondary". The GUI is the entry point of the application for your users, I would have made it the main module.
Then you can put routes.py
into its own webapp
(or whatever name fits its purpose more) folder. This will allow you to keep everything related to the "Web" part of your application in a single place. Try to at least separate your routes from your utilities functions.
You should also try to name your modules relative to what they do overall. main.py
is not a good name.
New layout
pyfladesk.py
import sys
from PyQt4.QtCore import QThread, QUrl
from PyQt4.QtGui import QApplication
from PyQt4.QtWebKit import QWebView
PORT = 5000
ROOT_URL = 'http://localhost:{}'.format(PORT)
class FlaskThread(QThread):
def __init__(self, application):
QThread.__init__(self)
self.application = application
def __del__(self):
self.wait()
def run(self):
self.application.run(port=PORT)
def provide_GUI_for(application):
qtapp = QApplication(sys.argv)
webapp = FlaskThread(application)
webapp.start()
qtapp.aboutToQuit.connect(webapp.terminate)
webview = QWebView()
webview.load(QUrl(ROOT_URL))
webview.show()
return qtapp.exec_()
if __name__ == '__main__':
from webapp.routes import app
sys.exit(provide_GUI_for(app))
webapp/routes.py
from flask import Flask, render_template
app = Flask(__name__)
@app.route('/')
def index():
return render_template('index.html')
The few more changes I made contains:
- improved naming a bit (not so much, you might come up with better names too);
- converting magic values to constants so they can easily be spotted and changed;
- used the callable
webapp.terminate
directly instead of creating alambda
.
-
\$\begingroup\$ WoW. Thanks for amazing review. I have some question. why do you import
from webapp.routes import app
afterif__ name__....
section? is it good idea to createconfig.py
for storing constants such asPORT
andROOT_URL
\$\endgroup\$Saeed M.– Saeed M.2015年12月18日 19:41:52 +00:00Commented Dec 18, 2015 at 19:41 -
\$\begingroup\$ @Mr-Moqadam Because I can. Well, it's mostly a bad habbit of mine, I tend to load things that are only needed when running the script from the command line (e.g. argparse) at this place instead of the top of the file. And for
config.py
, don't over-split. For now it is not needed at all. You'll be able to add it latter if it turns out that constants are an unmanageable mess otherwise. \$\endgroup\$301_Moved_Permanently– 301_Moved_Permanently2015年12月19日 13:06:50 +00:00Commented Dec 19, 2015 at 13:06