11
\$\begingroup\$

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:

  1. create a form including a QWebview widget
  2. create a Flask app
  3. run Flask app on QWebview

It works correctly but I want to know:

  1. How can I improve my code?
  2. 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')
\$\endgroup\$
8
+50
\$\begingroup\$

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 NameErrors.

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 a lambda.
\$\endgroup\$
  • \$\begingroup\$ WoW. Thanks for amazing review. I have some question. why do you import from webapp.routes import app after if__ name__.... section? is it good idea to create config.py for storing constants such as PORT and ROOT_URL \$\endgroup\$ – Saeed M. Dec 18 '15 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\$ – Mathias Ettinger Dec 19 '15 at 13:06

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.