mein Symbian S60 Python gewinn berechnungs programm

[nico_el_rey]

ich arbeite seit einiger Zeit an einem kleinem Gewinn-Berechnungs-Programm für meinen Laden
welches auf meinem Symbian-Handy laufen soll,
allerdings weiß ich selbst dass der Code recht
Chaotisch ist musste es allerdings wegen regelmäßigen global und anderen fehlern (nach return hatt dass Programm selbstständig gestoppt ( ohne Fehlerangabe) usw...)
nun läuft es jedenfalls endlich.
Um verbesserungsvorschläge wäre ich jedenfalls sehr dankbar *zwinker*
Code:

edit: sorry, hier downloadlink
und die Datei hatt auch mehr erklärende kommentare

http://www.file-upload.net/download-334 ... -5.py.html

mfg Nico

[Dauerbaustelle]

Die Seite funktioniert bei mir nicht. Kannst du das mal auf http://paste.pocoo.org stellen?

[BlackJack]

@nico_el_rey: Die Form ist gruselig, und da wirst Du wahrscheinlich mit dem Argument kommen, dass man Texte bei einem Mobiltelefon so schlecht eingeben kann -- deshalb auch meine Bedenken auf so einer Plattform zu entwickeln. Die ganzen abgekürzten Namen sollte man ausschreiben, mehr Leerzeichen zur besseren Lesbarkeit setzen und üblich sind vier Leerzeichen pro Einrückebene. Es gibt Leute die Kommentare lieber in eine extra Zeile vor dem Quelltext setzen, den sie erklären. Wenn man sie schon "inline" schreibt, sollte man das wenigstens nicht so unregelmässig weit abgesetzt tun. Das sieht sehr unruhig aus.

Wenn man die Namen nicht abkürzt, braucht man sie oft nicht in einem extra Kommentar erklären, weil sie selbst dann schon sagen wofür sie stehen. Wenn man die `gew()`-Funktion gleich `gewinn_berechnen()` nennt, braucht man den Kommentar nicht, der sagt, das es sich um die Gewinnberechnung handelt. Andere Namen hast Du zwar mit einem Kommentar versehen, aber es wird immer noch nicht klar was sie bedeuten. Zum Beispiel `pre`, `bra`, `bro`, und `zig` mit dem Kommentar ``# unicode angabe für ausgabetext, sonst globalfehler``. Häh!?

Die Daten für die Produkte sollten nicht an jeweils eigene Namen gebunden, und damit hart und ziemlich verteilt in den Programmcode eingebunden werden. Die sollten in einer geeigneten Datenstruktur stehen, so dass man ganz einfach an *einer* Stelle die Produkte erweitern oder verändern kann. Jetzt muss man sich ja neue Namen ausdenken und das gesamte Programm durchgehen, wenn man zum Beispiel ein Produkt hinzufügen möchte.

`running` ist ein Wahrheitswert und sollte dementsprechend an `True` und `False` und nicht an 1 und 0 gebunden werden.

Die einzelnen Punkte in der Hauptschleife machen zum Grossteil fast das selbe. 0 bis 3 könnte man jeweils zu einem einfachen Funktionsaufruf erledigen, der den gemeinsamen Quelltext enthält (Frage nach Anzahl, Gewinnberechnung, Ausgabe für Benutzer) und nur die Produktdaten als Argument übergeben bekommt.

Das zusammensetzen von Zeichenketten kann man mit Zeichenkettenformatierung über den ``%``-Operator oder die `format()`-Methode auf Zeichenketten lesbarer gestalten als durch ``+`` und `str()` -- noch dazu ohne Leerzeichen aneinander gequetscht.

Die Ausgaben in eine Datei überschreibt Werte wenn das Programm an einem Datum mehrfach gestartet wird. Das Format ist auch ungünstig, weil man es schlecht wieder einlesen und weiterverarbeiten kann. Es ist auch Abhängig davon welche Menüpunkte man vorher verwendet hat, was in der Datei landet und was nicht. Wenn man zwar Produkte verkauft hat, aber sich die Gesamtrechnung nicht anzeigen liess, wird deren Ergebnis nicht gespeichert. Wobei die Gesamtberechnung in einer vernünftigen Datendatei auch nicht nötig wäre, denn man könnte sie ja aus den Daten jederzeit neu ermitteln.

``for i in range(len(obj))`` ist in Python fast immer unnötig. Man kann direkt über die Elemente in `obj` iterieren statt den Umweg über einen Index zu machen. Wobei man sich für das Ersetzen von einem Zeichen in einer Zeichenkette eher keine eigene Schleife schreiben würde. Dafür haben Zeichenketten eine Methode. Man könnte mit `time.strftime()` aber auch gleich die Datumsangabe so formatieren lassen, wie man sie haben möchte.

Das zusammenfügen von Pfaden erledigt man besser mit `os.path.join()` statt ``+``.

Wozu ist `ausg` da? Da werden immer fleissig Texte hinzugefügt, aber es wird nie irgendwo verwendet!?

[daemonTutorials]

Habe mir mal die Mühe gemacht, und den Quellcode auf http://paste.pocoo.org hochgeladen.
http://paste.pocoo.org/show/368410/

Meine Mängel:
Wieso 'running' als global definieren?

mach es doch so:

Code: Alles auswählen

def quit():
    appuifw.note(u"ende")
    running = False

Und am Start des Programms gleich die Variable auf 'True' setzen. (Pseudocode)

Code: Alles auswählen

# Imports ...
running = True

# Alles weitere...

def quit():
    appuifw.note(u"ende")
    running = False

# Noch mehr...

while running:
    # Deine Abfragen

Ach mir fällt gerade ein. Du kannst bei 'eingabe == 6' das Programm wie folgt beenden, dazu brauchst du keine Funktion.

Code: Alles auswählen

elif eingabe == 6:
    appuifw.note(u"ende")
    break

Damit verlässt du die Schleife. Außerdem, 'running' ist global verfügbar.

Ich habe mir mal die Mühe gemacht, und alles mal 'leserlich' geschrieben: http://paste.pocoo.org/show/368424/

[Dauerbaustelle]

Habe mir mal die Mühe gemacht, und den Quellcode auf http://paste.pocoo.org hochgeladen.
http://paste.pocoo.org/show/368410/

Meine Mängel:
Wieso 'running' als global definieren?

mach es doch so:

Code: Alles auswählen

def quit():
    appuifw.note(u"ende")
    running = False

Und am Start des Programms gleich die Variable auf 'True' setzen. (Pseudocode)

Code: Alles auswählen

# Imports ...
running = True

# Alles weitere...

def quit():
    appuifw.note(u"ende")
    running = False

# Noch mehr...

while running:
    # Deine Abfragen

Das geht nicht. Du kannst eine globale Variable innerhalb einer Funktion nur unter Benutzung von `global` ändern.

Ich habe mir mal die Mühe gemacht, und alles mal 'leserlich' geschrieben: http://paste.pocoo.org/show/368424/

Das ist ja aber auch voll von Codewiederholung und außerdem, seh ich da Camelcase? Ihhhh

[BlackJack]

@daemonTutorials: Wenn `running` nicht als ``global`` deklariert ist, dann kannst Du es auf Modulebene auch nicht innerhalb von `quit()` setzen. Das funktioniert also so nicht, wie Du Dir das denkst.

Genausowenig wie Deine verbesserte Variante läuft, weil da ganz offensichtlich die Einrückung im Hauptteil nicht korrekt ist.

In lesbarer und auch ungetestet, könnte man das vielleicht so schreiben:

Code: Alles auswählen

# -*- coding: utf-8 -*-
import os
import sys
from functools import partial
from time import strftime

import appuifw

SAVE_PATH = 'c:/Laden/'

PRODUKTE = [
    {'name': n, 'einkaufspreis': e, 'verkaufspreis': v, 'gewinn': 0}
    for n, e, v in [
        ('Presidente', 54.375, 70),
        ('Brahma', 37.5, 50),
        ('Bohemian', 46.875, 60),
        ('Zigaretten', 4.25, 5),
    ]
]

WAEHRUNG = u'pesos'


def verkaufen(produkt):
    anzahl = appuifw.query(u'Wieviel wurden verkauft?', 'number')
    gewinn = anzahl * (produkt['einkaufspreis'] - produkt['verkaufspreis'])
    produkt['gewinn'] += gewinn
    appuifw.note(
        u'\nDer Gewinn von %s = %.2f %s' % (produkt['name'], gewinn, WAEHRUNG)
    )


def gesamtgewinn_ausgeben(produkte):
    appuifw.note(
        u'\nGesamtgewinn = %.2f %s'
        % (sum(p['gewinn'] for p in produkte), WAEHRUNG)
    )


def speichern(produkte):
    filename = os.path.join(SAVE_PATH, strftime('%Y-%m-%dT%H%M%S'))
    with open(filename, 'w') as save_file:
        save_file.writelines(
            '%s;%.2f\n' % (p['name'], p['gewinn']) for p in produkte
        )
    appuifw.note(
        u'Die Rechnung wurde unter\n'
        u' dem heutigen Datum gesichert'
    )


def main():
    menu = [(p['name'], partial(verkaufen, p)) for p in PRODUKTE]
    menu.extend(
        [
            ('Gesamt', partial(gesamtgewinn_ausgeben, PRODUKTE)),
            ('Speichern', partial(speichern, PRODUKTE)),
            ('Ende', sys.exit)
        ]
    )
    auswahl = [x[0] for x in menu]
    funktionen = [x[1] for x in menu]
    while True:
        eingabe = appuifw.popup_menu(auswahl, u'Was soll berechnet werden?')
        funktionen[eingabe]()

[nico_el_rey]

Oo... Erstmal vielen Dank fuer due vielen Reaktionen,
anregungen, verbesserungsvorschläge, verbesserungen, umschreibungen usw.

Ihr habt mir dass komplette Programm verbessert und verkuerzt.
Ich selbst habe die halbe Nacht dranngesessen und
schon einige verbesserungen selbstständig durchgeführt, kommt allerdings bei weitem nicht an euer Know How ran! *shame*

paste.pocoo.org/show/368561/

zudem Arbeite ich gerade an einer komplett neuen Version mit Klassen-system die allerdings noch nicht
"Posting-reif" ist, werde sie denke ich morgen über
paste.pocoo.org online stellen.

Übrigens gibt es auf Python S60 leider kein
- functools Modul
*leider*
beim speichern mit ctime-Datum besteht übrigens keine Gefahr mit überschreiben da die Uhrzeit mit angegeben ist *zwinker*
hierbei auch vielen Dank für strftime

so macht lernen richtig spaß *freu

mfg Nico

[daemonTutorials]

@dauerbaustelle: Das geht doch:

Code: Alles auswählen

>>> running = True
>>> def test():
...     print("ende")
...     running = False
... 
>>> test()
ende
>>> 

[deets]

@daemonTutorial

Wenn du schon sowas machst, dann solltest du es auch richtig machen:

Code: Alles auswählen

>>> running = True
>>> def test():
...     running = False
...     print "test"
... 
>>> test()
test
>>> print running
True

Was du gezeigt hast ist, dass eine Funktion einen eigenen Namensraum hat, in dem du durchaus Namen haben kannst, die auch global existieren.

Aber nicht, dass ohne spezielle Deklaration eine globale Variable von einer Funktion aus beschrieben wird.

[nico_el_rey]

@daemonTutorial

Wenn du schon sowas machst, dann solltest du es auch richtig machen:

Code: Alles auswählen

>>> running = True
>>> def test():
...     running = False
...     print "test"
... 
>>> test()
test
>>> print running
True

Was du gezeigt hast ist, dass eine Funktion einen eigenen Namensraum hat, in dem du durchaus Namen haben kannst, die auch global existieren.

Aber nicht, dass ohne spezielle Deklaration eine globale Variable von einer Funktion aus beschrieben wird.

also lag ich mit global doch nicht falsch, wusst ich natürlich weil ichs vorher ohne hatte *lach* (Endlosschleife *grrr*)

so, aktueller stand der dinge:
Laden - Klassen (Verion 1).py
http://paste.pocoo.org/show/368624/

Kritik erwünscht will ja schlieslich dazulernen und verbessern

bin gerade dabei zu Versuchen eine möglichkeit zu finden die verkaufsliste mittels append zu erweitern und den neuen Artikel direkt automatisch zu initiieren, muss dafür allerdings wieder einiges ändern aber dass is halt so *grins*

mfg Nico

[Dauerbaustelle]

Deine Variablennamen sind immer noch nichtssagend. `eingabe` kannst du rauswerfen, wird gar nicht verwendet. `artikel=""` ist auch nutzlos, weil der Name nur innerhalb der Methode gültig ist.

Ich würde die Artikel in einer Liste speichern, also z.B.

Code: Alles auswählen

artikel = [
    Verkaufsgegenstand('Foo', ...),
    Verkaufsgegenstand('Bar', ...),
    ...
]

Daraus kannst du dann das Auswahlmenü erzeugen und die Abfrage der Indices (`if artikel == n`) sparen und einfach `artikel[n]` verwenden.

`ende()` kannst du dir ganz sparen, indem du einfach `exit()` aufrufst, wodurch auch das `weiter` unnötig wird. Für das Dateilesen/schreiben solltest du das `with`-Statement verwenden.

[cofi]

Code: Alles auswählen

weiter = True
def ende():
    global weiter
    appuifw.note(u"auf wiedersehen")
    weiter = False

Das ist relativ sinnfrei. Du benoetigst den Code an genau einer Stelle, in der Hauptschleife, und dort laesst es sich weit sinnvoller mit einer Endlosschleife und `break` loesen - oder mit dem Flag aber eben nicht innerhalb einer Funktion.

Code: Alles auswählen

eingabe =""
class verkaufsgegenstand:
    global eingabe

Eh? Was? Das macht keinen Sinn, vor allem nicht wenn man es nicht verwendet.

Code: Alles auswählen

# Presidente
ekPres = 55
vkPres = 70

Das schreit direkt nach einem Tupel oder einer eigenen Klasse oder du laesst es ganz Weg und verwendest nur deine Klasse. Die Redundanz ist unnoetig.

Mal exemplarisch eine methode:

Code: Alles auswählen

def gewinnberechnung(self):
        self.anz= appuifw.query(u"wieviele wurden verkauft? ", "number")
        self.gewinn = self.anz*(self.vk-self.ek)
        self.gesVK= self.anz*self.vk
        self.gesEK = self.anz*self.ek
        appuifw.note(u"der Gewinn von %s ist %i" % (self.name, self.gewinn))
        artikel =""

`artikel` ist eine lokale Variable, es ist sinnlos die am Ende zu definieren, weil sie direkt danach geloescht wird.
Du vermischt Interface mit Logik, das solltest du trennen, zb so:

Code: Alles auswählen

def gewinnberechnung(self, anzahl):
        self.gewinn = self.anz*(self.vk-self.ek)
        self.gesVK= self.anz*self.vk
        self.gesEK = self.anz*self.ek
        return self.gewinn

und die Ausgabe am Ort des Aufrufs erledigen. Da geht aber noch mehr: `self.anz`, `self.gewinn`, `self.ges*` sind alles Daten die fuer eine Transaktion gueltig sind, es ist ueberfluessig bis schaedlich (weil die Daten inkonsistent werden) sie im Objekt zu speichern.

Und du solltest dir auf jeden Fall PEP 8 durchlesen.

[nico_el_rey]

zitat von cofi:
"""
Code:
# Presidente
ekPres = 55
vkPres = 70
Das schreit direkt nach einem Tupel oder einer eigenen Klasse oder
du laesst es ganz Weg und verwendest nur deine Klasse. Die
Redundanz ist unnoetig.
"""
Da sich einkaufs und verkaufspreiße ständig ändern wollte ich so eine einfache änderung ermöglichen

zitat cofi:"""
Code:
eingabe =""
class verkaufsgegenstand:
global eingabe
Eh? Was? Das macht keinen Sinn, vor allem nicht wenn man es
nicht verwendet.
"""
ich geb dir recht, hatte teilweise das problem dass die menuabfrage nicht funktioniert hatt und dass programm nach einem durchlauf recht unkontrolliert durch die menüs gehüpft ist, wollte ich damit lösen, war allerdings unnötig, der fehler lag an einem = , hab danach vergessen den Mist wieder zu löschen, sorry.

Zitat von Dauerbaustelle: """
Deine Variablennamen sind immer noch nichtssagend. `eingabe`
kannst du rauswerfen, wird gar nicht verwendet. `artikel=""` ist
auch nutzlos, weil der Name nur innerhalb der Methode gültig ist.
Ich würde die Artikel in einer Liste speichern, also z.B.
Code:
artikel = [
Verkaufsgegenstand('Foo', ...),
Verkaufsgegenstand('Bar', ...),
...
]
Daraus kannst du dann das Auswahlmenü erzeugen und die
Abfrage der Indices (`if artikel == n`) sparen und einfach `artikel[n]`
verwenden.
`ende()` kannst du dir ganz sparen, indem du einfach `exit()`
aufrufst, wodurch auch das `weiter` unnötig wird. Für das
Dateilesen/schreiben solltest du das `with`-Statement verwenden.
"""
werd ich direkt übernehmen, Danke
für den Tip, habe mit dieser Methode noch nie gearbeitet.

mfg Nico

[Dauerbaustelle]

Da sich einkaufs und verkaufspreiße ständig ändern wollte ich so eine einfache änderung ermöglichen

Ist doch wurscht, an welche Stelle du jetzt die Zahlen änderst :-)

[daemonTutorials]

@Dauerbaustelle: Also deswegen haben bei mir die Programme nicht richtig funktioniert. Da nehme man doch besser eine Klasse. Dort geht das nämlich ohne weiteres(wenn man kein _ oder __ davor hat)

[nico_el_rey]

so Leuz, hab noch ein wenig verändert,
VerkaufsListe kann jetzt auch erweitert werden und die berechnungen funktionieren auch gut,

mein Problem ist dass ich jetzt dass speichern nicht mehr hinbekomme,
kann mir bitte jemand etwas auf die sprünge helfen?

Aktueller Code:
http://paste.pocoo.org/show/370392/

ich habe hier irgendeinen fehler in der Speicherfunktion, fange ihn zwar mit try auf, komm aber nicht weiter...

Mfg Nico

[BlackJack]

@nico_el_rey: Dann "behandle" die Aunahme doch mal einfach *nicht*, dann sollte auch eine vernünftige Meldung kommen was genau das Problem ist. "funzt nicht" ausgeben ist nicht besonders hilfreich.

[EyDu]

Und damit hast du dir den Grund vor Augen geführt, warum nur die Ausnahmen abgefangen werden sollten, welche auch behandelt werden können. Sonst werden Fehler blind geschluckt und treten an irgend einer anderen Stelle auf, da sich das Programm in keinem gültigen Zustand mehr befindet. Noch ein paar weitere Hinweise:

- ``elif`` existiert
- benutze das ``with``-Statement
- Pfade werden mit ``os.path.join`` zusammengesetzt
- unterteile dein Programm in Funktionen
- ändere dein Programm so, dass du ohne ``exec`` auskommst
- schaue dir PEP 8 an
- ``while``-Schleifen kann man mit ``break`` verlassen, damit sparst du das ``weiter``

[Dauerbaustelle]

Was mich wundert, ist, warum nico_el_rey nach Verbesserungsvorschlägen fragt, sie aber nicht umsetzt.

[nico_el_rey]

so, meine Probleme:
code:
for i in artikelMenu(artikelMenu):
art = u"\nder gewinn von %s = %f" % (artikelMenu, artikelmenu.anzeigen())

fehlerausgabe : name i not defined
... Häh? Bei: for i in range(5): muss ich i doch auch nicht definieren ( hab "artikel gegen i getauscht)

code (jetzt ohne exec(...):
if auswahl == 1:
artikel = appuifw.popup_menu(artikelMenu, u"von was?")
artikelMenu[artikel].anzeigen()
fehlerausgabe:
Traceback (most recent call last):
File "ped.py", line 1366, in run_click
File "D:\Ped.temp\laden - Klasse [Version 2].py", line 76, in ?
artikelMenu[artikel].anzeigen()
AttributeError: 'unicode' object has no attribute 'anzeigen'

so, ich gebs bald auf, mit Klassensystem komm ich nich klar :K