Hallo,
ich möchte mich gleich anhängen in einer kritischen Code-Begutachtung.
Ich bin ebenfalls ein Neueinsteiger
(Hoffentlich mache ich micht nicht unbeliebt, indem ich Euch "nötige",
dass Ihr meine Hausaufgaben korrigiert )
Ich habe (rudimentär) versucht, das Tool "Agent Group Order" in Python und Wxpython
nachzuschreiben (Listing am Ende).
Welche grundlegenden Code (Struktur) Verbesserungen könnte dieses Programm
haben?
BTW: Welchen Editor verwendet ihr?
(ich weiss, (x)emacs oder vim wäre optimal, aber ich aus der (noch) Windows
Welt kann mich an sie schwer eingewöhnen)
Autovervollständigung (bzw. Vorschläge), Identation automatisch erkennen,
Python Hilfe leicht aufrufbar (vom Editor) wäre optimal.
Die normale Python Hilfe, soweit ich weiss, gibt es nur als html Datei.
Bei Active-Python kann ich sie auch als .chm herunterladen.
Am liebsten wäre mir das hlp Format, da ich auf diese Hilfe auch direkt in
Multi-Edit zugreifen kann.
Beste Grüsse,
-----------------------------------------------------------------------------
from wxPython.wx import *
from string import *
import operator
class MyFrame(wxFrame):
def __init__(self, parent, title):
#frame init
wxFrame.__init__(self, parent, -1, title, size=(500, 450))
#create menu
menuBar = wxMenuBar()
menu = wxMenu()
menu.Append(101, "E&xit\tAlt-X", "Exit demo")
EVT_MENU(self, 101, self.OnButtonClose)
menuBar.Append(menu, "&File")
self.SetMenuBar(menuBar)
#panel und buttons
self.panel = wxPanel(self, -1)
self.btn_write = wxButton(self.panel, -1, "Write", wxPoint(280, 140))
self.btn_write.SetDefault()
self.btn_up = wxButton(self.panel, -1, "Up", wxPoint(280, 60))
self.btn_up.SetDefault()
self.btn_down = wxButton(self.panel, -1, "Down", wxPoint(280, 100))
self.btn_down.SetDefault()
def read_and_display_groups (self):
self.groupList = []
self.subs_groupList = []
dat_array = []
#bind Button Methods
EVT_BUTTON(self, self.btn_write.GetId(), self.OnButtonWrite)
EVT_BUTTON(self, self.btn_up.GetId(), self.OnButtonUp)
EVT_BUTTON(self, self.btn_down.GetId(), self.OnButtonDown)
#filelength and entries
input_idx.seek (0, 2)
idx_bytes = input_idx.tell()
idx_entries = input_idx.tell() /16
input_idx.seek (0, 0)
#grouplist
dlg_read_in = wxProgressDialog("Reading dat file", "Status", 3, self.panel)
for line in input_dat.readlines():
self.groupList.append (line)
#idx array
dlg_read_in.Update(2, "Read Idx File")
self.idx_string = input_idx.read ()
dlg_read_in.Destroy()
dlg_process = wxProgressDialog("Process ...", "Status", 100, self.panel)
old = -1
hundredth = idx_bytes / 100
if hundredth == 0:
hundredth = 1
for index in range (0, idx_bytes, 16):
if index / hundredth != old:
old = index / hundredth;
dlg_process.Update (old, str (old) + ' %')
#extract flags
subscribed = 0
if operator.and_ (ord (self.idx_string [index+3]), 0x80):
subscribed = 1
if operator.and_ (ord (self.idx_string [index+3]), 0x04):
pass
if operator.and_ (ord (self.idx_string [index+3]), 0x08):
pass
dat_array.append (self.idx_string [index + 11])
#subscribed grouplist
if subscribed:
self.subs_groupList.append (replace(self.groupList [index/16], "\n", ""))
dlg_process.Destroy()
self.lb1 = wxListBox(self.panel, 60, wxPoint(80, 50), wxSize(180, 250),
self.subs_groupList, wxLB_SINGLE)
#append index for sorting
for i in range (0, len (self.subs_groupList)):
self.subs_groupList = "%6d" % i + self.subs_groupList + "\n"
#bindings for listbox
EVT_LISTBOX(self, 60, self.EvtListBox)
EVT_LISTBOX_DCLICK(self, 60, self.EvtListBoxDClick)
self.lb1.SetSelection(0)
self.btn_down.SetFocus ()
def OnButtonWrite(self, evt):
output_dat = open ("groups1.dat", 'w')
output_idx = open ("groups1.idx", 'wb')
dat_file_ptr = 0
idx_ptr = 0;
current_entry = 0;
for line in self.subs_groupList:
#write one entry of subscribed list in groups.dat
output_dat.write (line [6:])
i = dat_file_ptr
b_0 = i / 2**16
i = i % 2**16
b_1 = i / 2**8
i = i % 2**8
b_2 = i
#extract index
index = int (line [:6])
temp_str = chr (b_2) + chr (b_1) + chr (b_0) + self.idx_string [index * 16 + 3:index * 16 + 16]
#write one entry of subscribed list in groups.idx
output_idx.write (temp_str);
dat_file_ptr = dat_file_ptr + len (line) - 5; #wegen cr, lf ist ein zeichen?
current_entry += 1
#write rest (is unchanged)
temp_entry = 0
for line in self.groupList:
temp_entry += 1
if temp_entry > current_entry:
output_dat.write (line)
output_idx.write (self.idx_string [current_entry*16:])
output_dat.close ()
output_idx.close ()
wxMessageBox("Written", "Ready", wxOK)
def OnButtonClose(self, evt):
self.Close()
def OnButtonUp(self, evt):
#swaps positon in subs_grouplist and directly in listbox
selected = self.lb1.GetSelection()
selected_string = self.lb1.GetString(selected)
self.lb1.Delete(selected)
i = self.subs_groupList.pop (selected)
self.lb1.InsertItems ([selected_string], selected -1)
self.subs_groupList.insert (selected - 1, i)
self.lb1.SetSelection (selected-1)
#button up/down enable/disable and focus
if selected-1 == 0:
self.btn_up.Enable (FALSE)
self.btn_down.SetFocus ()
else:
self.btn_up.Enable (TRUE)
self.btn_down.Enable (TRUE)
def OnButtonDown(self, evt):
#see above
selected = self.lb1.GetSelection()
selected_string = self.lb1.GetString(selected)
self.lb1.Delete(selected)
i = self.subs_groupList.pop (selected)
self.lb1.InsertItems ([selected_string], selected + 1)
self.subs_groupList.insert (selected + 1, i)
self.lb1.SetSelection (selected+1)
if selected+1 == self.lb1.Number () - 1:
self.btn_down.Enable (FALSE)
self.btn_up.SetFocus ()
else:
self.btn_down.Enable (TRUE)
self.btn_up.Enable (TRUE)
def OnButtonClose(self, evt):
self.Close()
def EvtListBox(self, event):
#directly select in list box
#button up/down enable/disable and focus
if self.lb1.GetSelection() == self.lb1.Number () - 1:
self.btn_down.Enable (FALSE)
else:
self.btn_down.Enable (TRUE)
if self.lb1.GetSelection() == 0:
self.btn_up.Enable (FALSE)
self.btn_down.SetFocus ()
else:
self.btn_up.Enable (TRUE)
if self.lb1.GetSelection() == self.lb1.Number () - 1:
self.btn_up.SetFocus ()
def EvtListBoxDClick(self, event):
pass
app = wxPySimpleApp()
input_dat = open ("groups.dat", 'r')
input_idx = open ("groups.idx", 'rb')
frame = MyFrame(None, "(Freeware) F.St. Utility for Agent")
frame.Show(true)
frame.read_and_display_groups ()
app.MainLoop()
-----------------------------------------------------------------------------
Kritik an meinem ersten kleinen Pyth/Wxpyth prog erwünscht
-
- User
- Beiträge: 728
- Registriert: Sonntag 22. September 2002, 08:32
- Wohnort: Sauerland
- Kontaktdaten:
erster Eindruck: sieht sauber aus. Einrückung schwach. Du solltest bei Sourcecode immer die Atrtribute [ code ] ...... [ /code ] verwenden. Damit wird auf eine Schriftart mit fester Weite umgeschaltet, z. B. Courier.
Versuch doch mal einen vi für Windump, z. B. bei http://www.softwareonline.org/products.html. Wie bei Windowsprogrammen üblich, Kostet der aber. Ich finde aber, es lohnt sich.
Mach ich doc mal ein Probelauf mit deinem Programm
Hans
Versuch doch mal einen vi für Windump, z. B. bei http://www.softwareonline.org/products.html. Wie bei Windowsprogrammen üblich, Kostet der aber. Ich finde aber, es lohnt sich.
Mach ich doc mal ein Probelauf mit deinem Programm
Hans
Hallo Hans,
danke erstmals für Deine prompte Antwort,
ich probiere es nochmals (habe vorhin den code einfach mit copy/paste eingefügt):
also code:
danke erstmals für Deine prompte Antwort,
ich probiere es nochmals (habe vorhin den code einfach mit copy/paste eingefügt):
also code:
Code: Alles auswählen
from wxPython.wx import *
from string import *
import operator
class MyFrame(wxFrame):
def __init__(self, parent, title):
#frame init
wxFrame.__init__(self, parent, -1, title, size=(500, 450))
#create menu
menuBar = wxMenuBar()
menu = wxMenu()
menu.Append(101, "E&xit\tAlt-X", "Exit demo")
EVT_MENU(self, 101, self.OnButtonClose)
menuBar.Append(menu, "&File")
self.SetMenuBar(menuBar)
#panel und buttons
self.panel = wxPanel(self, -1)
self.btn_write = wxButton(self.panel, -1, "Write", wxPoint(280, 140))
self.btn_write.SetDefault()
self.btn_up = wxButton(self.panel, -1, "Up", wxPoint(280, 60))
self.btn_up.SetDefault()
self.btn_down = wxButton(self.panel, -1, "Down", wxPoint(280, 100))
self.btn_down.SetDefault()
#process groups.dat and groups.idx
def read_and_display_groups (self):
self.groupList = []
self.subs_groupList = []
dat_array = []
#bind Button Methods
EVT_BUTTON(self, self.btn_write.GetId(), self.OnButtonWrite)
EVT_BUTTON(self, self.btn_up.GetId(), self.OnButtonUp)
EVT_BUTTON(self, self.btn_down.GetId(), self.OnButtonDown)
#filelength and entries
input_idx.seek (0, 2)
idx_bytes = input_idx.tell()
idx_entries = input_idx.tell() /16
input_idx.seek (0, 0)
#grouplist
dlg_read_in = wxProgressDialog("Reading dat file", "Status", 3, self.panel)
for line in input_dat.readlines():
self.groupList.append (line)
#idx array
dlg_read_in.Update(2, "Read Idx File")
self.idx_string = input_idx.read ()
dlg_read_in.Destroy()
#dlg_process = wxProgressDialog("Process ...", "Status", idx_bytes, self.panel)
dlg_process = wxProgressDialog("Process ...", "Status", 100, self.panel)
old = -1
hundredth = idx_bytes / 100
if hundredth == 0:
hundredth = 1
for index in range (0, idx_bytes, 16):
if index / hundredth != old:
old = index / hundredth;
dlg_process.Update (old, str (old) + ' %')
#extract flags
subscribed = 0
if operator.and_ (ord (self.idx_string [index+3]), 0x80):
subscribed = 1
if operator.and_ (ord (self.idx_string [index+3]), 0x04):
pass
if operator.and_ (ord (self.idx_string [index+3]), 0x08):
pass
dat_array.append (self.idx_string [index + 11])
#subscribed grouplist
if subscribed:
self.subs_groupList.append (replace(self.groupList [index/16], "\n", ""))
dlg_process.Destroy()
self.lb1 = wxListBox(self.panel, 60, wxPoint(80, 50), wxSize(180, 250),
self.subs_groupList, wxLB_SINGLE)
#append index for sorting
for i in range (0, len (self.subs_groupList)):
self.subs_groupList [i] = "%6d" % i + self.subs_groupList [i] + "\n"
#bindings for listbox
EVT_LISTBOX(self, 60, self.EvtListBox)
EVT_LISTBOX_DCLICK(self, 60, self.EvtListBoxDClick)
self.lb1.SetSelection(0)
self.btn_down.SetFocus ()
def OnButtonWrite(self, evt):
output_dat = open ("groups1.dat", 'w')
output_idx = open ("groups1.idx", 'wb')
dat_file_ptr = 0
idx_ptr = 0;
current_entry = 0;
for line in self.subs_groupList:
#write one entry of subscribed list in groups.dat
output_dat.write (line [6:])
i = dat_file_ptr
b_0 = i / 2**16
i = i % 2**16
b_1 = i / 2**8
i = i % 2**8
b_2 = i
#extract index
index = int (line [:6])
temp_str = chr (b_2) + chr (b_1) + chr (b_0) + self.idx_string [index * 16 + 3:index * 16 + 16]
#write one entry of subscribed list in groups.idx
output_idx.write (temp_str);
dat_file_ptr = dat_file_ptr + len (line) - 5; #wegen cr, lf ist ein zeichen?
current_entry += 1
#write rest (is unchanged)
temp_entry = 0
for line in self.groupList:
temp_entry += 1
if temp_entry > current_entry:
output_dat.write (line)
output_idx.write (self.idx_string [current_entry*16:])
output_dat.close ()
output_idx.close ()
wxMessageBox("Written", "Ready", wxOK)
def OnButtonClose(self, evt):
self.Close()
def OnButtonUp(self, evt):
#swaps positon in subs_grouplist and directly in listbox
selected = self.lb1.GetSelection()
selected_string = self.lb1.GetString(selected)
self.lb1.Delete(selected)
i = self.subs_groupList.pop (selected)
self.lb1.InsertItems ([selected_string], selected -1)
self.subs_groupList.insert (selected - 1, i)
self.lb1.SetSelection (selected-1)
#button up/down enable/disable and focus
if selected-1 == 0:
self.btn_up.Enable (FALSE)
self.btn_down.SetFocus ()
else:
self.btn_up.Enable (TRUE)
self.btn_down.Enable (TRUE)
def OnButtonDown(self, evt):
#see above
selected = self.lb1.GetSelection()
selected_string = self.lb1.GetString(selected)
self.lb1.Delete(selected)
i = self.subs_groupList.pop (selected)
self.lb1.InsertItems ([selected_string], selected + 1)
self.subs_groupList.insert (selected + 1, i)
self.lb1.SetSelection (selected+1)
if selected+1 == self.lb1.Number () - 1:
self.btn_down.Enable (FALSE)
self.btn_up.SetFocus ()
else:
self.btn_down.Enable (TRUE)
self.btn_up.Enable (TRUE)
def OnButtonClose(self, evt):
self.Close()
def EvtListBox(self, event):
#directly select in list box
#button up/down enable/disable and focus
if self.lb1.GetSelection() == self.lb1.Number () - 1:
self.btn_down.Enable (FALSE)
else:
self.btn_down.Enable (TRUE)
if self.lb1.GetSelection() == 0:
self.btn_up.Enable (FALSE)
self.btn_down.SetFocus ()
else:
self.btn_up.Enable (TRUE)
if self.lb1.GetSelection() == self.lb1.Number () - 1:
self.btn_up.SetFocus ()
def EvtListBoxDClick(self, event):
pass
app = wxPySimpleApp()
input_dat = open ("groups.dat", 'r')
input_idx = open ("groups.idx", 'rb')
frame = MyFrame(None, "(Freeware) F.St. Utility for Agent")
frame.Show(true)
frame.read_and_display_groups ()
app.MainLoop()
-
- Python-Forum Veteran
- Beiträge: 2010
- Registriert: Freitag 11. Oktober 2002, 18:00
- Wohnort: Salzburg
- Kontaktdaten:
Hallo Francesco,
schaut ja ganz schön aus, allerdings hätte ich noch ein paar Anmerkungen.
Du verwendest unterschiedliche Eintückungstiefen, ich hoffe Du mischt nicht Leerzeichen(Space) und Tabulatoren(Tabs). Ansonst verwende am besten immer 4 Leerzeichen zum Einrücken. Gute Editoren lassen sich so einstellen, daß sie immer Leerzeichen verwenden.
Auch der Teil:
lässt sich einfacher als:
schreiben.
Auch machst Du manchmal zwischen Funktions-/Methodennamen und Klammerauf ein Leerzeichen und dann wieder nicht, was das Lesen des Codes etwas erschwert. Ich selber bevorzuge die Schreibweise ohne Leerzeichen, aber es gibt auch andere Programmierer, die, die Klammer als Parametertuplebegrenzung sehen und ein Leerzeichen bevorzugen. IMHO sollte beides in Ordnung sein, aber halt nicht beides in einem Code.
Jedenfalls für Dein erstes Python-Programm echt gut.
Gruß
Dookie
schaut ja ganz schön aus, allerdings hätte ich noch ein paar Anmerkungen.
Du verwendest unterschiedliche Eintückungstiefen, ich hoffe Du mischt nicht Leerzeichen(Space) und Tabulatoren(Tabs). Ansonst verwende am besten immer 4 Leerzeichen zum Einrücken. Gute Editoren lassen sich so einstellen, daß sie immer Leerzeichen verwenden.
Auch der Teil:
Code: Alles auswählen
if self.lb1.GetSelection() == self.lb1.Number () - 1:
self.btn_down.Enable (FALSE)
else:
self.btn_down.Enable (TRUE)
Code: Alles auswählen
self.btn_down.Enable (self.lb1.GetSelection() != self.lb1.Number () - 1)
Auch machst Du manchmal zwischen Funktions-/Methodennamen und Klammerauf ein Leerzeichen und dann wieder nicht, was das Lesen des Codes etwas erschwert. Ich selber bevorzuge die Schreibweise ohne Leerzeichen, aber es gibt auch andere Programmierer, die, die Klammer als Parametertuplebegrenzung sehen und ein Leerzeichen bevorzugen. IMHO sollte beides in Ordnung sein, aber halt nicht beides in einem Code.
Jedenfalls für Dein erstes Python-Programm echt gut.
Gruß
Dookie
Hallo Dookie,
danke für das Lob,
Mein erstes Python Programm ist eigentlich nicht wirklich,
aber erstes bisschen größeres.
Ich habe auch Teile von Surce code "geklaut" über das WxPyhton Demo von mehreren
Programmen und die Python Sources an sich durchsucht, um möglichst schnell
zu einem Ergebnis zu kommen.
Zur Einrückung:
ich vewende (wie auch in C/C++) eigentlich nur Spaces und eine Einrückung von
2 Zeichen. Ich hasse es, wenn wer 4 oder gar 8 einrückt
Die 4 Zeichen kommen nur vom Copy/Paste.
Das mit den Klammer fällt mir erst jetzt auf, ja das erschwert sicher die
Lesbarkeit.
Viele Grüsse
danke für das Lob,
Mein erstes Python Programm ist eigentlich nicht wirklich,
aber erstes bisschen größeres.
Ich habe auch Teile von Surce code "geklaut" über das WxPyhton Demo von mehreren
Programmen und die Python Sources an sich durchsucht, um möglichst schnell
zu einem Ergebnis zu kommen.
Zur Einrückung:
ich vewende (wie auch in C/C++) eigentlich nur Spaces und eine Einrückung von
2 Zeichen. Ich hasse es, wenn wer 4 oder gar 8 einrückt
Die 4 Zeichen kommen nur vom Copy/Paste.
Das mit den Klammer fällt mir erst jetzt auf, ja das erschwert sicher die
Lesbarkeit.
Viele Grüsse
-
- User
- Beiträge: 728
- Registriert: Sonntag 22. September 2002, 08:32
- Wohnort: Sauerland
- Kontaktdaten:
Ich habe mir das auch mal angesehen (extra wxPython installiert).
Ich mag es nicht, wenn die GUI komplett im Hauptprogramm entworfen wird. Mag daran Liegen, dass ich normanl mit Delphi arbeite und auf Linux mit QT angefangen habe. Da werden die Daten für das Fenster immer in einer eigenen Datei (Klasse) gespeichert und man leitet diese nur noch ab. Bei komplizierten Fensteraufbauten erhöht das die Lesbarkeit.
Mir ist nicht klar, wofür die Indexdatei benötigt wird und wie diese arbeitet (habe mir aber auch noch nicht die Mühe gemacht, das verstehen zu wollen). Kleine Datenmengen kann man mühelos sequentiell sortieren und bei großen Mengen würde ich wenn eben möglich über eine Datenbank gehen. In einer SQL-Abfrage kann man dann ganz schnell mit order by feldname eine Sortierung erzwingen.
Hans
Ich mag es nicht, wenn die GUI komplett im Hauptprogramm entworfen wird. Mag daran Liegen, dass ich normanl mit Delphi arbeite und auf Linux mit QT angefangen habe. Da werden die Daten für das Fenster immer in einer eigenen Datei (Klasse) gespeichert und man leitet diese nur noch ab. Bei komplizierten Fensteraufbauten erhöht das die Lesbarkeit.
Mir ist nicht klar, wofür die Indexdatei benötigt wird und wie diese arbeitet (habe mir aber auch noch nicht die Mühe gemacht, das verstehen zu wollen). Kleine Datenmengen kann man mühelos sequentiell sortieren und bei großen Mengen würde ich wenn eben möglich über eine Datenbank gehen. In einer SQL-Abfrage kann man dann ganz schnell mit order by feldname eine Sortierung erzwingen.
Hans
Hallo Hans,hans hat geschrieben:Ich habe mir das auch mal angesehen (extra wxPython installiert).
hast Du extra wegen meiner Frage wxPython installiert?
In einem anderen Thread habe ich gesehen, dass ich diese Frage besser
im GUI Teil des Forums posten sollte, aber ich wollte nur eine Kritik des Programmstils an sich.
Äh, habe ich mir noch nicht angeschaut, aber ich gebe Dir völlig recht.hans hat geschrieben: Ich mag es nicht, wenn die GUI komplett im Hauptprogramm entworfen wird. Mag daran Liegen, dass ich normanl mit Delphi arbeite und auf Linux mit QT angefangen habe. Da werden die Daten für das Fenster immer in einer eigenen Datei (Klasse) gespeichert und man leitet diese nur noch ab. Bei komplizierten Fensteraufbauten erhöht das die Lesbarkeit.
Ich möchte in den nächsten Tagen sowieso mal den Boa-Constructor
anschauen. BTW: (nein, ich stelle die Frage jetzt im GUI Forum)
Ist auch nicht so wichtig; Forte Agent speichert die Eigenschaften der Gruppenliste etwas umständlich in der index Datei ab.hans hat geschrieben: Mir ist nicht klar, wofür die Indexdatei benötigt wird und wie diese arbeitet (habe mir aber auch noch nicht die Mühe gemacht, das verstehen zu wollen).