Mój pierwszy program python: czy możesz mi powiedzieć, co robię źle?

Question

Mam nadzieję, że to pytanie zostanie uznane za odpowiednie dla stackoverflow. Jeśli nie, natychmiast usuwam to pytanie.

Właśnie napisałem mój pierwszy program python. Pomysł polega na tym, że możesz wydać polecenie i zostanie ono wysłane do kilku serwerów równolegle.

Jest to przeznaczone wyłącznie do osobistych celów edukacyjnych. Program działa! Naprawdę chcę się poprawić w Pythonie i dlatego chciałbym zadać następujące pytania:

1. Mój styl wygląda bałagan w porównaniu do PHP (do czego jestem przyzwyczajony). Czy masz jakieś sugestie dotyczące ulepszeń stylu?
2. Czy używam poprawnych bibliotek? Czy używam ich poprawnie?
3. Czy używam poprawnych typów danych? Czy używam ich poprawnie?

Mam dobre przygotowanie programistyczne, ale zajęło mi sporo czasu, aby opracować przyzwoity styl dla PHP (standardy kodowania PEAR, wiedząc, jakich narzędzi użyć i kiedy).

Źródłem (jeden plik, 92 linii kodu)

http://code.google.com/p/floep/source/browse/trunk/floep

Answer 1

Zwykle jest korzystne, co następuje po zakończeniu kary : jest w osobnej linii (również nie spację przed to)

if options.verbose: 
    print "" 

zamiast

if options.verbose : print "" 

Nie trzeba sprawdzić len z listy, jeśli idziesz do iteracyjne nad nim

if len(threadlist) > 0 : 
    for server in threadlist : 
    ... 

jest zbędny, bardziej „czytelny” jest (python jest wystarczająco inteligentny, aby nie iteracyjne nad pustą listę) :

for server in threadlist: 
    ... 

również bardziej 'pythonistic' jest użycie listowych lista'S (ale na pewno jest dyskusyjna opinia)

server = [] 
for i in grouplist : servers+=getServers(i) 

można skrócić do

server = [getServers(i) for i in grouplist] 

Answer 2

wyjątki String są przestarzałe w Pythonie, więc ta linia:

if not config.has_section(sectionname): 
    raise 'Server or group ' + sectionname + ' not found in ' + configfile 

powinny być przerobione na coś takiego:

if not config.has_section(sectionname): 
    raise ConfigNotFoundError(
     "Server or group" + sectionname + "not found in" + configfile) 

class ConfigNotFoundError(Exception): 
    pass 

[Edytowane w celu odzwierciedlenia sugestię dangph w komentarzach ]

To więcej linii kodu, ale lepiej dla przyszłych uaktualnień.

Przez wzgląd na czytelność, w coś takiego:

parser.add_option('-q','--quiet',action="store_false", help="Display only server output", dest="verbose", default=True) 

może być zapisane tak:

parser.add_option('-q', 
        '--quiet', 
        action="store_false", 
        help="Display only server output", 
        dest="verbose", 
        default=True) 

może wolisz inną metodę podziału sposób przywołać, ale chodzi o to, że długo linie mogą być trudne do odczytania.

Powinieneś także przeczytać PEP 8, aby poznać styl Pythona.

Answer 3

Przed rozładowaniem krytyki najpierw pozwól mi powiedzieć gratulacje z okazji uruchomienia Twojego pierwszego programu w Pythonie. Przemieszczanie się z jednego języka na drugi może być uciążliwe, ciągle grzebiąc w kwestiach składniowych i polując przez nieznane biblioteki.

Najbardziej wymienianą wytyczną stylu jest PEP-8, ale to tylko poradnik, a przynajmniej jej część jest ignorowana ... nie, mam na myśli uznawana za nie stosującą się do jakiejś konkretnej sytuacji z całym szacunkiem dla autorów wytycznych i współtwórcy :-).

Nie mogę porównać tego z PHP, ale w porównaniu z innymi aplikacjami Python jest całkiem jasne, że postępujesz zgodnie z konwencjami stylu z innych języków. Nie zawsze zgadzałem się z wieloma rzeczami, które inni deweloperzy powiedzieli ci, abyś zrobił to, ale z czasem zrozumiałem, dlaczego używanie konwencji pomaga w informowaniu o tym, co robi aplikacja, i pomoże innym programistom ci pomóc.

---

Podnoszenie wyjątków, a nie ciągi.

raise 'Server or group ' + sectionname + ' not found in ' + configfile

staje

raise RuntimeError('Server or group ' + sectionname + ' not found in ' + configfile)

---

Nie spację przed „:” w końcu za „czy” lub „za”, a nie umieszczać wielu instrukcji w tej samej linii, a bądź konsekwentny w kwestii umieszczania spacji wokół operatorów. Używać nazw zmiennych dla obiektów i trzymać i i j dla zmiennych indeks pętli (jak nasze mistrzowskie FORTRAN przodków):

for i in grouplist : servers+=getServers(i)

staje:

for section in grouplist: 
    servers += getServers(section)

---

Kontenery mogą być testowane pod kątem zawartości bez uzyskania ich długość:

while len(threadlist) > 0 :

zostaje

while threadlist:

i

if command.strip() == "" :

staje

if command.strip():

---

Dzielenie krotka nie jest zazwyczaj umieścić w nawiasie po stronie lewej oświadczenia, a logika komenda jest nieco zawiłe . Jeśli nie ma argumentów, to "" .join (...) Będzie pusty łańcuch:

 
(options,args) = parser.parse_args() 

if options.verbose : print "floep 0.1" 

command = " ".join(args) 

if command.strip() == "" : parser.error('no command given') 

staje

 
options, args = parser.parse_args() 
if options.verbose: 
    print "floep 0.1" 

if not args: 
    parser.error('no command given') 
command = " ".join(args) 

---

Python dla pętli niezwykły „else” klauzuli, która jest wykonywana wtedy, gdy pętla przechodzi przez wszystkie elementy bez a 'break':

 
    for server in threadlist : 
     foundOne = False 
     if not server.isAlive() : 
      ...snip... 
      foundOne = True 

     if not foundOne : 
      time.sleep(0.010) 

staje

 
    for server in threadlist: 
     if not server.isAlive(): 
      ...snip... 
      break 
    else: 
     time.sleep(0.010) 

---

Getting listę linii, a następnie łącząc je razem jest trochę za długa zdyszany:

 
     result = proc.readlines() 
     strresult = '' 
     for line in result : strresult+=line 
     self.result = strresult 

staje

 
     self.result = proc.read() 

---

Korzystanie Biblioteka jest dobry, sprawdź podproces moduł, jest trochę bardziej aktualny.

Twoje typy danych są w porządku.

a dostaniesz mnóstwo innych anwsers :-)

Answer 4

Często w celu ponownego wykorzystania, mamy następujące, zaczynając od około linia 48 w programie

def main(): 
    config = ConfigParser.RawConfigParser() 
    etc. 

if __name__ == "__main__": 
    main() 

To tylko punkt wyjścia.

Gdy to zrobisz, zdasz sobie sprawę, że main() to naprawdę dwie części: analiza interfejsu wiersza poleceń i wykonanie pracy. Następnie chcesz refaktoryzować rzeczy tak wyglądać.

def serverWork(group,...): 
    servers = getServers(group) 
    etc. 

def main(): 
    config = ConfigParser.RawConfigParser() 

    if command.strip() == "": 
     parser.error('no command given') 
    else: 
     serverWork(options.group, options.etc., ...) 

Teraz podniosłeś prawdziwą pracę do funkcji w tym module. Twoja funkcja serwera może teraz być ponownie wykorzystana przez inne programy lub skrypty.