Aufbau Skript - Sinn und Unsinn von Klassen

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
Benutzeravatar
martinjo
User
Beiträge: 189
Registriert: Dienstag 14. Juni 2011, 20:03

Hallo,

ich bin gerade wieder mal dabei, ein altes Skript zu überarbeiten. Ursprünglich waren das nur ein paar lose zusammengesetzte Funktionen. Nun möchte ich etwas Struktur hinein bringen.

Da ich hier in letzter Zeit öfters mal mitbekommen habe, dass über Sinn und Unsinn von Klassen gesprochen wurde ("Du willst keine Klassen nutzen ;-)") dachte ich es ist evtl. von Vorteil gleich zu beginn meines Skriptes hier nachzufragen.

Es handelt sich um ein Skript, welches Bestellungen aus der MySQL-Datenbank eines Webshops ausliest. Und mir ist aufgefallen, dass ich alleine zum auslesen der Einstellungen eine eigene Klasse geschrieben habe.

Das Skript ist nun auch wieder nur Teil eines großen Ganzen welches ich gerade umsetze. Die Daten werden spätes durch ein anderes Skript weiterverarbeitet und dann in eine Warenwirtschaft importiert.

Ich freue mich wie immer über Kommentare :-)

Code: Alles auswählen

#!/usr/bin/python
# -*- coding: utf8 -*-

import os
import logging
import datetime
import MySQLdb
import MySQLdb.cursors 
from decimal import Decimal
from pprint import pprint
from ConfigParser import SafeConfigParser


CONFIGFILE_WEBSHOP          = os.path.join (os.path.expanduser("~"), ".config/python_webshop_db_config.ini")
CONFIGFILE_WEBSHOP_SECTION  = "data"
CONFIGFILE_WEBSHOP_OPTIONS  = ["webshop_host", "webshop_user", "webshop_pass", "webshop_db"]


class MyConfigfile():
    
    def __init__(self):
        logging.basicConfig(level=logging.DEBUG, format='%(levelname)s:\t%(message)s')
        self.configfile_parser = SafeConfigParser()

    
    def write_configfile(self, filename, section, options):
        with open(filename , "w") as configfile:
            logging.info("writing config file ...")
            self.configfile_parser.add_section(section)
            for option in options:
                self.configfile_parser.set(section, option, "{0}".format(raw_input("please enter {0}: ".format(option))))
            self.configfile_parser.write(configfile)
           
    def update_configfile(self, filename, section, options):
        with open(filename, "r+") as configfile:
            logging.info("updating config file ...")
            if not section in self.configfile_parser.sections():
                self.configfile_parser.add_section(section)
            for option in options:
                self.configfile_parser.set(section, option, "{0}".format(raw_input("please enter {0}: ".format(option))))
            self.configfile_parser.write(configfile)
           
    def read_configfile(self, filename, section, options):
        logging.info("reading config file {}".format(filename))
        self.configfile_parser.read(filename)
        while not section in self.configfile_parser.sections():
            logging.warning("config file corrupt, writing new one ...")
            self.write_configfile(filename, section, options)
        for option in options:
            if not self.configfile_parser.has_option("data", option):
                logging.warning("option {0} is missing".format(option))
            if not self.configfile_parser.get("data", option):
                logging.warning("option {0} has no value".format(option))
                self.update_configfile(options=[option])
                self.read_configfile(filename, section, options)
                
    def run(self):             
        if os.path.exists(CONFIGFILE_WEBSHOP):
            logging.debug("configfile exists")
            self.read_configfile(CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
        else:
            logging.warning("configfile not exists")
            self.write_configfile(CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
            self.read_configfile(CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
            
        mysql_opts = {
            'host': self.configfile_parser.get("data", "webshop_host"),
            'user': self.configfile_parser.get("data", "webshop_user"),
            'pass': self.configfile_parser.get("data", "webshop_pass"),
            'db'  : self.configfile_parser.get("data", "webshop_db")
            }
        return mysql_opts



class GetModifiedWebshopSales():
    
    def __init__(self):
        logging.basicConfig(level=logging.DEBUG, format='%(levelname)s:\t%(message)s')        
        
    def connect_to_database(self, mysql_opts="" ): 
        if not mysql_opts:
            mysql_opts = MyConfigfile().run()
        logging.debug("connection to MySQL database")                   
        mysql = MySQLdb.connect(mysql_opts['host'], mysql_opts['user'], mysql_opts['pass'], mysql_opts['db'], cursorclass=MySQLdb.cursors.DictCursor, charset='utf8')
        mysql.apilevel = "2.0"
        mysql.threadsafety = 2
        mysql.paramstyle = "format"
        self.cursor = mysql.cursor()

    def get_orders(self, state="processing", number=""):
        logging.debug("get orders")
        orders = []
        # by state
        if state:
            logging.info("Import order(s) by state")
            logging.info("State = {}".format(state))
            order_states = { "open"         : "1",
                             "finished"     : "3",
                             "processing"   : "2" }
            # validate state
            if state in ["processing"]:
                self.cursor.execute("SELECT * FROM `orders` WHERE `orders_status` = %s", order_states[state])
                orders = self.cursor.fetchall()
                logging.info("orders found: {}".format(len(orders)))            
        # by number
        elif number:
            logging.info("Import order(s) by number")
            logging.info("Number = {}".format(number))
            cursor.execute("SELECT * FROM `orders` WHERE `orders_id` = %s", i)
            orders = cursor.fetchall()
            logging.info("orders found: {}".format(len(orders)))            
        else:
            logging.info("No import type statement found")
            exit("No import type statement found")
            
            
        # for order in orders
        for order in orders:
            pprint (order)
            orders_id = order["orders_id"]
            order["customers_name"]

            order_template = {
                "order_description" : "",
                "order_total"       : Decimal(0),
                "order_state"       : "",

                "order_sale_lines"       : {},
                "order_party"    : {},
                "order_shipping_address" : {},
                "order_payments"         : {}
                
                }
    
            orders_id = order["orders_id"]
            
            # party name
            order_template["order_party"]["name"]    = order["customers_name"]
            order_template["order_party"]["company"] = order["customers_company"]
            order_template["order_party"]["street"]  = order["billing_street_address"]
            order_template["order_party"]["street2"] = order["delivery_suburb"]
            order_template["order_party"]["zip"]     = order["customers_postcode"]
            order_template["order_party"]["city"]    = order["customers_city"]
            order_template["order_party"]["country"] = order["customers_country"]
            order_template["order_party"]["email"]   = order["customers_email_address"]
            order_template["order_party"]["phone"]   = order["customers_telephone"]
            order_template["order_party"]["vat"]     = order["customers_vat_id"]
            
            
            order_template["order_shipping_address"]["name"]    = order["delivery_name"]
            order_template["order_shipping_address"]["company"] = order["delivery_company"]
            order_template["order_shipping_address"]["street"]  = order["billing_street_address"]
            order_template["order_shipping_address"]["street2"] = order["delivery_suburb"]
            order_template["order_shipping_address"]["zip"]     = order["delivery_postcode"]
            order_template["order_shipping_address"]["city"]    = order["delivery_city"]
            order_template["order_shipping_address"]["country"] = order["delivery_country"]
            
            
            # get products / order lines
            order_lines = []
            self.cursor.execute("SELECT * FROM `orders_products` WHERE `orders_id` = %s", order["orders_id"])
            orders_lines = self.cursor.fetchall()
            logging.info("lines in order: {}".format(len(orders_lines)))
            for order_line in orders_lines:
                line = {}
                line["product_name"]     = order_line["products_name"]
                line["product_code"]     = order_line["products_model"]
                line["product_price"]    = order_line["products_price"]
                line["product_quantity"] = order_line["products_quantity"]
                
                
            #self.cursor.execute("SELECT * FROM `orders_total` WHERE `orders_id` = %s", order["orders_id"])
            #orders_total = self.cursor.fetchall()
            #pprint (orders_total)
            
            

            
            
def main():
    base = GetModifiedWebshopSales()
    base.connect_to_database()
    base.get_orders()

if __name__ == "__main__":
    main()
Zuletzt geändert von Anonymous am Freitag 20. November 2015, 18:11, insgesamt 1-mal geändert.
Grund: Quelltext in Python-Codebox-Tags gesetzt.
Sirius3
User
Beiträge: 18309
Registriert: Sonntag 21. Oktober 2012, 17:20

@martinjo: was mir auf die schnelle auffällt:

os.path.expanduser funktioniert mit ganzen Pfaden, das join ist unnötig.
Der Unterschied zwischen write_configfile und update_configfile leuchtet mir nicht ein, pack das in einer Funktion zusammen.
"r+" ist der falsche Filemode.
MyConfigfile.run hat einen schlechten Namen.
In GetModifiedWebshopSales werden nicht alle Attribute in __init__ gesetzt.
Ein cursor ist etwas kurzlebiges und sollte nicht als Attribut gespeichert werden.
Das prophylaktische Setzen von Variablen, die danach gleich wieder überschrieben werden, solltest Du sein lassen. Dadurch kann man Fehler viel schwieriger finden.
Was sollen eigentlich die Dictionary-Umkopier-Orgien?
Dictioneries belegt man gleich beim Erzeugen mit richtigen Werten und nicht gleich dahinter per Item-Setting.
Benutzeravatar
MagBen
User
Beiträge: 799
Registriert: Freitag 6. Juni 2014, 05:56
Wohnort: Bremen
Kontaktdaten:

In der Objektorientierung werden Funktionen mit den dazugehörigen Daten verknüpft. Diese mit ihren Daten verknüpften Funktionen werden deshalb Methoden genannt. Klassen sind ein Hilfsmittel um diese Verknüpfung von Daten und Funktionen möglichst einfach und syntaktisch klar zu erreichen. Grundsätzlich funktioniert Objektorientierung aber auch mit Sprachen, die gar keine Klassen unterstützen, z.B. mit C, es ist einfach nur wesentlich aufwändiger und braucht mehr Code.

Umgekehrt kann man auch prozedural mit Klassen programmieren. Deine Klassen haben 0-1 Membervariablen, d.h. die Verbindung von Daten und Funktionen in Klassen ist fast gar nicht vorhanden. Deine Objekte sind Zustandslos. Du programmierst also prozedural (das ist nicht abwertend gemeint). Wenn Du die Klassen weglassen würdest, würde Dein Code genauso gut funktionieren, aber der Code wäre klarer. Du würdest eine Einrücktiefe sparen und die ganzen unnötigen self.
a fool with a tool is still a fool, www.magben.de, YouTube
Benutzeravatar
martinjo
User
Beiträge: 189
Registriert: Dienstag 14. Juni 2011, 20:03

Hallo und Danke für eure Antworten. Ich werde mal versuchen auf alles einzugehen.
os.path.expanduser funktioniert mit ganzen Pfaden, das join ist unnötig.
Bei einem absoluten Pfad müsste ich hier ja den Benutzernamen angeben (/home/benutzer/...). Das habe ich nicht anders hin bekommen.
Der Unterschied zwischen write_configfile und update_configfile leuchtet mir nicht ein, pack das in einer Funktion zusammen.
Mit update_configfile soll es möglich sein, nur einzelne Werte die fehlen nach zu tragen. Z.b. wenn später noch ein zusätzlicher Wert in der Konfogfile abgespeichert werden soll, dann soll nur nach dem fehlenden Wert gefragt werden und dieser ergänzt werden.
"r+" ist der falsche Filemode.
Danke für den Hinweis. Ist da w+ besser? Ich will mit dieser Funktion ja nur Werte ergänzen, die noch nicht vorhanden sind.
MyConfigfile.run hat einen schlechten Namen.
Ok, werde ich anpassen. Habe ich ohne groß zu überlegen gewählt.
In GetModifiedWebshopSales werden nicht alle Attribute in __init__ gesetzt.
Welche Attribute meinst du? Soll ich alles wo mit self anfängt hier schon erstellen? Also z.B. auch (abgesehen davon dass curser hier nicht definiert werden soll) self.cursor = None ?
Ein cursor ist etwas kurzlebiges und sollte nicht als Attribut gespeichert werden.
ok, werde ich anpassen. Dann werde ich zu beginn der Klassenfunktion get_orders() verbinden.
Das prophylaktische Setzen von Variablen, die danach gleich wieder überschrieben werden, solltest Du sein lassen. Dadurch kann man Fehler viel schwieriger finden.
Ja, dass sollte ich langsam mal berücksichtigen. Für mich scheint es meist noch etwas übersichtlicher es in mehreren Schritten zu machen da ich noch nicht ganz so fit bin im code lesen. Und gerade wenn es dann verschachtelt ist lege ich manchmal lieber 2-3 Zeilen an als es in einer zu machen :oops: .
Was sollen eigentlich die Dictionary-Umkopier-Orgien?
Dass ist vor allem temporär zur Hilfe, damit ich sehe was ich noch alles auslesen muss. Das alte Skript sah ja ganz anders aus.
Dictioneries belegt man gleich beim Erzeugen mit richtigen Werten und nicht gleich dahinter per Item-Setting.
Direkt am Anfang mit den passenden Werten anlegen geht leider nicht, da die Werte oft geprüft und manipuliert werden müssen. Ich könnte es jedoch ganz zum Schluss anlegen wenn ich alle passenden Werte schon habe.
Umgekehrt kann man auch prozedural mit Klassen programmieren. Deine Klassen haben 0-1 Membervariablen, d.h. die Verbindung von Daten und Funktionen in Klassen ist fast gar nicht vorhanden. Deine Objekte sind Zustandslos. Du programmierst also prozedural (das ist nicht abwertend gemeint). Wenn Du die Klassen weglassen würdest, würde Dein Code genauso gut funktionieren, aber der Code wäre klarer. Du würdest eine Einrücktiefe sparen und die ganzen unnötigen self.
Ich habe die Klassen eigentlich der Übersicht wegen gewählt :-) Auch damit ich in dem ausführenden Skript nur eine Klasse importieren muss.

Dann werde ich mal den Code etwas anpassen und später hier die neue Version veröffentlichen.
Benutzeravatar
cofi
Python-Forum Veteran
Beiträge: 4432
Registriert: Sonntag 30. März 2008, 04:16
Wohnort: RGFybXN0YWR0

Was mit dem `expanduser` Kommentar gemeint war ist folgendes:

Code: Alles auswählen

In [2]: os.path.expanduser('~/this/is/a/path/')
Out[2]: '/home/cofi/this/is/a/path/'
BlackJack

@martinjo: Du brauchst doch aber trotzdem kein `join()` sondern kannst den Pfad gleich beim `expanduser()`-Aufruf angeben. Das wäre doch ziemlich sinnlos wenn man dort nur '~' angeben könnte, denn dann könnte man das Argument auch gleich weg lassen.

Der Dateimodus ist 'w'. Alles mit einem '+' ist fehlerhaft weil die ganze Datei neu geschrieben werden muss ohne das irgendwelcher Datenmüll am Ende bleibt wenn die Datei durch neue Werte kleiner werden sollte. Vielleicht liegt hier auch ein Verständnisproblem: Es werden *immer* alle Daten geschrieben, auch wenn Du nur einen Schlüssel hinzugefügt hast. Deswegen ist `write_configfile()` und `update_configfile()` auch so ”komisch”. Das sind keine zwei verschiedenen Funktionen, oder zumindest sollten die nicht vollständig ausprogrammiert sein weil die im Grunde fast das selbe machen.

In der `__init__()` sollten alle Attribute eingeführt werden. Das Objekt sollte danach vollständig initialisiert sein und man sollte daran ablesen können was alles zum Objektzustand gehört ohne alle Methoden durchlesen zu müssen.
Benutzeravatar
martinjo
User
Beiträge: 189
Registriert: Dienstag 14. Juni 2011, 20:03

Danke, ich bin jetzt leider ganz draußen aus meiner Struktur.

Was ich nun nach neuer Strukturierung tun möchte ist folgendes:

Die Daten aus dem Webshop auslesen, aufbereiten und weitergeben.
Der Vorgang soll aus einem zweiten Skript aufgerufen werden indem ich die Klasse GetModifiedWebshopSales() importiere und an der gewünschten Stelle ausführe. Optional möchte ich als Parameter mitgeben: "mysql_opts" falls eine andere Datenbank verwendet werden soll. Und "number" falls nur eine Bestellung ausgelesen werden soll.

Nun soll, falls kein Parameter "mysql_opts" mitgegeben wurde die Konfigurationsdatei geladen werden.

Lohnt sich dafür eine Klasse überhaupt? Ich mag Klassen weniger wegen den Vorteilen wie Vererbung (die ich noch gar nicht nutze) sondern weil ich es als Übersichtlicher empfinde.
Benutzeravatar
martinjo
User
Beiträge: 189
Registriert: Dienstag 14. Juni 2011, 20:03

martinjo hat geschrieben:Danke, ich bin jetzt leider ganz draußen aus meiner Struktur.

Was ich nun nach neuer Strukturierung tun möchte ist folgendes:

Die Daten aus dem Webshop auslesen, aufbereiten und weitergeben.
Der Vorgang soll aus einem zweiten Skript aufgerufen werden indem ich die Klasse GetModifiedWebshopSales() importiere und an der gewünschten Stelle ausführe. Optional möchte ich als Parameter mitgeben: "mysql_opts" falls eine andere Datenbank verwendet werden soll. Und "number" falls nur eine Bestellung ausgelesen werden soll.

Nun soll, falls kein Parameter "mysql_opts" mitgegeben wurde die Konfigurationsdatei geladen werden.

Lohnt sich dafür eine Klasse überhaupt? Ich mag Klassen weniger wegen den Vorteilen wie Vererbung (die ich noch gar nicht nutze) sondern weil ich es als Übersichtlicher empfinde.

edit:
hier als Beispiel der neu strukturierte noch nicht ganz überarbeitete Code:

Code: Alles auswählen

class GetModifiedWebshopSales():

    logging.basicConfig(level=logging.DEBUG, format='%(levelname)s:\t%(message)s')
    
    def __init__(self, mysql_opts=None, number=None, state="processing"):
        self.mysql_opts = self.read_write_config() if not mysql_opts else mysql_opts
        self.state = state
        self.number = number
        
    def read_write_config(self):
        
        CONFIGFILE_WEBSHOP          = os.path.expanduser("~/.config/python_webshop_db_config.ini")
        CONFIGFILE_WEBSHOP_SECTION  = "data"
        CONFIGFILE_WEBSHOP_OPTIONS  = ["webshop_host", "webshop_user", "webshop_pass", "webshop_db"]

                
        configfile_parser = SafeConfigParser()
        
        def write_configfile(self, filename, section, options):
            with open(filename , "w") as configfile:
                logging.info("writing config file ...")
                configfile_parser.add_section(section)
                for option in options:
                    configfile_parser.set(section, option, "{0}".format(raw_input("please enter {0}: ".format(option))))
                configfile_parser.write(configfile)
                   
        def update_configfile(self, filename, section, options):
            with open(filename, "r+") as configfile:
                logging.info("updating config file ...")
                if not section in configfile_parser.sections():
                    configfile_parser.add_section(section)
                for option in options:
                    configfile_parser.set(section, option, "{0}".format(raw_input("please enter {0}: ".format(option))))
                configfile_parser.write(configfile)
               
        def read_configfile(self, filename, section, options):
            logging.info("reading config file {}".format(filename))
            configfile_parser.read(filename)
            while not section in configfile_parser.sections():
                logging.warning("config file corrupt, writing new one ...")
                write_configfile(filename, section, options)
            for option in options:
                if not configfile_parser.has_option("data", option):
                    logging.warning("option {0} is missing".format(option))
                if not configfile_parser.get("data", option):
                    logging.warning("option {0} has no value".format(option))
                    update_configfile(options=[option])
                    read_configfile(filename, section, options)
                    
        if os.path.exists(CONFIGFILE_WEBSHOP):
            logging.debug("configfile exists")
            read_configfile(self ,CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
        else:
            logging.warning("configfile not exists")
            write_configfile(CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
            read_configfile(CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
            
        mysql_opts = {
            'host': configfile_parser.get("data", "webshop_host"),
            'user': configfile_parser.get("data", "webshop_user"),
            'pass': configfile_parser.get("data", "webshop_pass"),
            'db'  : configfile_parser.get("data", "webshop_db")
            }
        return mysql_opts
        
           

    def get_orders(self, mysql_opts="",  state="processing", number=""):
        logging.debug("get orders")
Sirius3
User
Beiträge: 18309
Registriert: Sonntag 21. Oktober 2012, 17:20

@martinjo: mach alles rückgängig. Der neue Code ist viel schlechter geworden. Funktionen innerhalb von Funktionen zu definieren, macht fast nie Sinn. Das macht ein Programm nur weniger Lesbar und gar nicht mehr testbar. Dass get_orders die selben Parameter hat, wie __init__ läßt auch nur schlimmes erahnen. Ein logging.basicConfig macht nur im Hauptprogramm Sinn, auf keinen Fall in einer Klassendefinition, da gehören wirklich nur Definitionen hin.
Benutzeravatar
martinjo
User
Beiträge: 189
Registriert: Dienstag 14. Juni 2011, 20:03

Ich war am überarbeiten, die doppelten Parameter sind weg. Es ist nicht schön. Aber es funktioniert nun und liefert meinem Programm die Werte die ich haben will. Auch die Konfigurationsdatei wird richtig angelegt und fehlende Werte ergänzt. Ich nehme die Hinweise von dir zur Kenntnis und werde versuchen dies beim nächsten Programm (Nun kommt auslesen der Artikel von eBay) besser umzusetzen.

Wie ich dass nun aber mit dem ConfigParser angehe stellt mich weiterhin vor ein Rätsel. Hatte das Problem ja auch hier schon mal angegangen: viewtopic.php?f=1&t=36950

Mit logging fange ich gerade erst an warm zu werden. Habe ich ins Hauptprogramm, Danke

Code: Alles auswählen

class GetModifiedWebshopSales():
    
    def __init__(self, mysql_opts=None, number=None, state="processing"):
        self.mysql_opts = self.read_write_config() if not mysql_opts else mysql_opts
        self.state = state
        self.number = number
        
    def read_write_config(self):
        
        CONFIGFILE_WEBSHOP          = os.path.expanduser("~/.config/python_webshop_db_config.ini")
        CONFIGFILE_WEBSHOP_SECTION  = "data"
        CONFIGFILE_WEBSHOP_OPTIONS  = ["webshop_host", "webshop_user", "webshop_pass", "webshop_db"]

                
        configfile_parser = SafeConfigParser()
        
        def write_configfile(self, filename, section, options):
            with open(filename , "w") as configfile:
                logging.info("writing config file ...")
                configfile_parser.add_section(section)
                for option in options:
                    configfile_parser.set(section, option, "{0}".format(raw_input("please enter {0}: ".format(option))))
                configfile_parser.write(configfile)
                   
        def update_configfile(filename, section, options):
            with open(filename, "w") as configfile:
                logging.info("updating config file ...")
                if not section in configfile_parser.sections():
                    configfile_parser.add_section(section)
                for option in options:
                    configfile_parser.set(section, option, "{0}".format(raw_input("please enter {0}: ".format(option))))
                configfile_parser.write(configfile)
               
        def read_configfile(self, filename, section, options):
            logging.info("reading config file {}".format(filename))
            configfile_parser.read(filename)
            while not section in configfile_parser.sections():
                logging.warning("config file corrupt, writing new one ...")
                write_configfile(filename, section, options)
            for option in options:
                if not configfile_parser.has_option("data", option):
                    logging.warning("option {0} is missing".format(option))
                if not configfile_parser.get("data", option):
                    logging.warning("option {0} has no value".format(option))
                    update_configfile(filename, section, options=[option])
                    read_configfile(self, filename, section, options)
                    
        if os.path.exists(CONFIGFILE_WEBSHOP):
            logging.debug("configfile exists: ok")
            read_configfile(self ,CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
        else:
            logging.warning("configfile not exists")
            write_configfile(CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
            read_configfile(CONFIGFILE_WEBSHOP, CONFIGFILE_WEBSHOP_SECTION, CONFIGFILE_WEBSHOP_OPTIONS)
            
        mysql_opts = {
            'host': configfile_parser.get("data", "webshop_host"),
            'user': configfile_parser.get("data", "webshop_user"),
            'pass': configfile_parser.get("data", "webshop_pass"),
            'db'  : configfile_parser.get("data", "webshop_db")
            }
        return mysql_opts
        
           

    def get_orders(self):
        logging.debug("get orders")
        mysql_opts = self.mysql_opts
        state = self.state
        number = self.number

        logging.debug("connection to MySQL database")                   
        mysql = MySQLdb.connect(mysql_opts['host'], mysql_opts['user'], mysql_opts['pass'], mysql_opts['db'], cursorclass=MySQLdb.cursors.DictCursor, charset='utf8')
        mysql.apilevel = "2.0"
        mysql.threadsafety = 2
        mysql.paramstyle = "format"
        cursor = mysql.cursor()

        orders = []
        # by state
        if state:
            logging.info("Import order(s) by state")
            logging.info("State = {}".format(state))
            order_states = { "open"         : "1",
                             "finished"     : "3",
                             "processing"   : "2" }
            order_states_by_number = { "1"         : "open",
                                       "3"     : "finished",
                                       "2"   : "processing" }
            # validate state
            if state in ["processing"]:
                cursor.execute("SELECT * FROM `orders` WHERE `orders_status` = %s", order_states[state])
                orders = cursor.fetchall()
                logging.info("orders found: {}".format(len(orders)))            
        
        # by number
        elif number:
            logging.info("Import order(s) by number")
            logging.info("Number = {}".format(number))
            cursor.execute("SELECT * FROM `orders` WHERE `orders_id` = %s", i)
            orders = cursor.fetchall()
            logging.info("orders found: {}".format(len(orders)))            
        else:
            logging.info("No import type statement found")
            exit("No import type statement found")
            
        # for order in orders
        for order in orders:
            orders_id = order["orders_id"]


            order_template = {
                "order_description"    : "Webshop {}".format(order["orders_id"]),
                "order_total"          : Decimal(0),
                "order_shipping_total" : Decimal(0),
                "order_state"          : order_states_by_number[str(order["orders_status"])],

                "order_sale_lines"       : [],
                "order_party"            : {},
                "order_shipping_address" : {},
                "order_payments"         : {}
                }

            
            # order party
            order_template["order_party"]["name"]    = order["customers_name"]
            order_template["order_party"]["company"] = order["customers_company"]
            order_template["order_party"]["street"]  = order["billing_street_address"]
            order_template["order_party"]["street2"] = order["delivery_suburb"]
            order_template["order_party"]["zip"]     = order["customers_postcode"]
            order_template["order_party"]["city"]    = order["customers_city"]
            order_template["order_party"]["country"] = order["customers_country"]
            order_template["order_party"]["email"]   = order["customers_email_address"]
            order_template["order_party"]["phone"]   = order["customers_telephone"]
            order_template["order_party"]["vat"]     = order["customers_vat_id"]
            
            # order shipping address
            order_template["order_shipping_address"]["name"]    = order["delivery_name"]
            order_template["order_shipping_address"]["company"] = order["delivery_company"]
            order_template["order_shipping_address"]["street"]  = order["billing_street_address"]
            order_template["order_shipping_address"]["street2"] = order["delivery_suburb"]
            order_template["order_shipping_address"]["zip"]     = order["delivery_postcode"]
            order_template["order_shipping_address"]["city"]    = order["delivery_city"]
            order_template["order_shipping_address"]["country"] = order["delivery_country"]
            
            
            # order sale lines
            cursor.execute("SELECT * FROM `orders_products` WHERE `orders_id` = %s", order["orders_id"])
            orders_lines = cursor.fetchall()
            logging.info("lines in order: {}".format(len(orders_lines)))
            for order_line in orders_lines:
                line = {}
                line["product_name"]     = order_line["products_name"]
                line["product_code"]     = order_line["products_model"]
                line["product_price"]    = order_line["products_price"]
                line["product_quantity"] = order_line["products_quantity"]
                order_template["order_sale_lines"].append(line)
                
            cursor.execute("SELECT * FROM `orders_total` WHERE `orders_id` = %s", order["orders_id"])
            orders_total = cursor.fetchall()
            
            for zeile in orders_total:
                # order total
                if zeile['class'] == "ot_total":
                    order_template["order_total"] = zeile["value"]
                    logging.debug("order total = {}".format(order_template["order_total"]))
                # shipping costs
                elif zeile['class'] == "ot_shipping":
                    shipping_costs_brutto = zeile["value"]
                    order_template["order_shipping_total"] = shipping_costs_brutto
                    logging.debug("shipping total = {}".format(order_template["order_shipping_total"]))

            return order_template
Zuletzt geändert von Anonymous am Sonntag 22. November 2015, 19:27, insgesamt 1-mal geändert.
Grund: Quelltext in Python-Codebox-Tags gesetzt.
Sirius3
User
Beiträge: 18309
Registriert: Sonntag 21. Oktober 2012, 17:20

@martinjo: Bei so Aussagen wie, "ich weiß dass es scheiße ist, Du hast ja recht, ist mir aber egal, und beim nächsten mal mach ich den gleichen Schrott wieder", könnt ich %$!?*#+$/.

Das was Du da abgeliefert hast, ist 1000mal schlechter als das am Anfang. Die angesprochenen Fehler sind immer noch drin, wie oft soll man denn das noch wiederholen? Und Du schreibst, es "funktioniert ja".

read_write_config ist keine Methode, weil self nicht vorkommt, die ganze Klasse ist unnötig, weil sie nur noch aus einer Methode besteht, wo man die paar Instanzattribute auch gleich als Parameter übergeben könnte.
Zeile 79: warum order vorinitialisieren? In welchen nachfolgenden Bedingungen wird order gesetzt und wann nicht? ist das richtig so?
Zeile 87: Sowas erzeugt man durchs programm und nicht händisch.
Zeile 91: WTF?
Zeile 105: exit innerhalb irgendwas außerhalb von main ist verboten. Das sollte eine Exception sein!
Zeile 112-144: Warum initilaisierst Du alles mit leeren Wörterbüchern und füllst sie nachträglich? Da wird auch nirgends eine komplizierte Rechnung gemacht, wie Du weiter oben behauptest!
Zeile 116: unintialisierte Variable, warum Strings als Keys, wenn es doch Zahlen sind?
Zeile 173: return innerhalb einer for-Schleife ist bestimmt nicht das, was Du willst!

Dein Datenbankentwurf sieht suboptimal aus. Die komplette Kundeninformation in der Tabelle orders? Die komplette Lieferaddresse in orders? Da muß es eine Tabelle customer und eine Tabelle delivery geben und in orders dürfen nur IDs auf diese Tabellen auftauchen!
orders_products enthält Produktname und -modell? Das sollte doch in einer Tabelle products stehen, und in orders_products nur eine ID auf diese Tabelle. Was soll die Tabelle orders_total? Eine Tabelle mit den Feldern class und value ist ja nicht besonders aussagekräftig.

Und Du willst mir immer noch sagen, das tut ja? Mindestens 4 schwere Fehler und etliche Designfehler!
Benutzeravatar
martinjo
User
Beiträge: 189
Registriert: Dienstag 14. Juni 2011, 20:03

@Sirius3: Nach deinem Text wollte ich das gleich heute nochmal überarbeiten, bin jedoch nicht dazu gekommen. Das zweite Skript wo ich schreibe ist vom Aufbau her identisch, daher dachte ich mache ich hier erst Mal weiter. Ich werde das Skript noch anpassen, ich wollte da gestern Abend fertig werden und habe ich dass vorschnell veröffentlicht und einige Fehler übersehen.

Für die Datenbankstruktur kann ich übrigens nichts, dass ist die modified eCommerce Shopsoftware :-)
Gondii
User
Beiträge: 24
Registriert: Freitag 11. September 2015, 05:56

Moin,

Ich wollte nur mal anmerken, dass ich beim lesen dieses Threads einiges über Klassen und guten Programmier-Stil gelernt habe. Also auch wenn Sirius hier offenbar die Hände über'm Kopf zusammen schlägt, ich hab aus den Antworten hier einiges für mich rausziehen können ;-)
Antworten