[Python-de] FileLight in Python + Tk

Stefan Schwarzer sschwarzer at sschwarzer.net
Mi Jul 2 21:11:24 UTC 2008


Hallo Mathias,

da sich sonst keiner erbarmt ;-) , gebe ich zumindest ein paar
allgemeine Hinweise. Ich kenne mich mit Tk nicht aus und
kann dir daher nur etwas zu Python im Allgemeinen sagen und
zu Englisch. ;-)

On 2008-06-27 14:19, Mathias Uebel wrote:
> ich habe mal wieder etwas zum _Auseinandernehmen_!
> Ich bin Autodidakt und habe also Niemanden, dem ich das Vorlegen kann.

Gibt es eine Benutzergruppe/User Group in deiner Umgebung,
deren Treffen du besuchen könntest?

> Darum bitte ich hier um Kritik, Anregungen ... Ich will etwas lernen.

> try:
> 	import sys, os
> 	from math import *

Die Form "from modul import *" solltest du im Allgemeinen
vermeiden, da du den Namensraum überfrachtest und dem folgenden
Code nicht mehr anzusehen ist, aus welchem Modul welcher
Bezeichner stammt.

> 	from Tkinter import *
> 	import Canvas
> except:
> 	print """Folgenden Module sind unbedingt erforderlich: ... """
> 	sys.exit(1)

Das Abfangen von Exceptions allein mit except solltest du
tunlichst vermeiden. Mit der allgemeinen Form gehst du ein
unnötiges Risiko ein, Ausnahmen zu verschleiern, die dich
interessieren sollten. Fange explizit nur die Ausnahmen ab,
die du erwartest und auf die du reagieren willst.

> # sort a list[list] as a model
> # aus dem forum http://www.unixboard.de/vb3/showthread.php?t=16795
> lcmp = lambda idx: lambda b, a: (a[idx] < b[idx]) and -1 or (a[idx] >
> b[idx]) and 1 or 0

Sieht meiner Ansicht nach schon "obfuscated" aus. ;-) Ich
würde eine längere aber lesbarere Funktion bevorzugen.

> def HumanValue(value):
> 	"""human-readable Values"""

Hauptwörter schreibt man im Englischen klein. Ein paar
Ausnahmen sind Eigennamen, Himmelsrichungen und Jahreszeiten
(unter bestimmten Bedingungen).

> 	if value > 1024 * 1024 * 1024:
> 		return "%d GB" % (value / 1024 / 1024 / 1024)
> 	if value > 1024 * 1024:
> 		return "%d MB" % (value / 1024 / 1024)
> 	if value > 1024:
> 		return "%d KB" % (value / 1024)
> 	return "%d B" % value

Hier gefällt mir gut, dass du die Vielfachen von 1024 deutlich
als solche hinschreibst statt Werte wie 1048576 zu verwenden.

> class PathWalk(object):
> 	"""walk the path recursiv and add the filesize"""
> 	def __init__(self, POINT=os.environ['HOME']): #getcwd()):
> 		self.POINT = POINT

Warum die eigenwillige Schreibung mit Großbuchstaben für ein
Attribut? Such mal nach PEP 8, dem Python Style Guide.

> 		self.oldPOINT = ""
> 		self.Action()
> 
> 	def Action(self):
> 		"""walk the File tree"""
> 		# only if is a new access
> [...]

Ich gebe zu, dass ich mir das jetzt nicht im Detail angeschaut
habe, aber ich würde erwarten, dass du hier mehr mit os.walk
machen könntest. Denk daran, dass du die durch den Iterator
zurückgegebenen Listen "in-place" verändern kannst (siehe
Python-Dokumentation).

> 	def FileListFiles(self):
> 		self.FILELIST_files.sort(cmp=lcmp(0))

Verwende am besten das Schlüsselwort-Argument key statt cmp.
Der einzige Grund, das nicht zu tun, wäre für mich, wenn der
Code auf einer relativ alten Python-Version laufen müsste.

> 	def FileSumme(self):

Den Namen FileSumme finde ich nicht sehr intuitiv. Außerdem
würde man in Python-Code eher file_summe schreiben. Hm, wenn
du versuchst, alles auf Englisch zu schreiben, warum nicht
"sum" statt "summe"?

> 	def Paint(self):
> 		"""Action"""

Kein sehr aussagekräftiger Docstring. ;-)

> 		# from filesize to 360 grad and 2pi

Auf Englisch sollte es "degrees" heißen, nicht "grad".

> 		self.NewFileList = []
> 		c = 0	# color value
> 		for file in self.FileList():
> 			self.NewFileList.append([	
> 					file[0], 							# filesize
> 					file[1], 							# name
> 					file[2], 							# mark 'd' or 'f'
> 					float((file[0] * 360)  / self.FileSumme()), 	# 100% = 360
> 					float((file[0] * 2 * pi) / self.FileSumme()),	# 100% = 2 x pi
> (Kreisumfang)
> 					Color[c],							# color
> 					''								#
> 					])

Warum rechnest du hier mit zwei verschiedenen Winkelmaßen?
360 Grad und 2*Pi sind doch äquivalent.

> 			c += 1
> 			# if end of color list: beginn the list
> 			if c == 24: c = 0;

Das Semikolon brauchst du nicht.

Soweit meine Kommentare nach einem recht kurzen Draufgucken.

Viele Grüße
Stefan



Mehr Informationen über die Mailingliste python-de