Script "schön" genug?

Sockets, TCP/IP, (XML-)RPC und ähnliche Themen gehören in dieses Forum
Antworten
Quakxi
User
Beiträge: 7
Registriert: Donnerstag 19. März 2015, 22:42

Das letzte mal haben sich ja einige über emine Schreibweise, zu Recht, beschwert.
Ich habe jetzt den Code noch einmal überarbeitet.
Wäre nett, wenn Ihr nochmal so Hilfsbereit sein könntet und von mir eventl. falsche/unschöne Sachen verbessert.
Der Code an sich funtzt.

Code: Alles auswählen

#!/usr/bin/env python
# coding: utf8

import socket
import pyglet
import os
import time

TCP_IP = '192.168.2.108'
TCP_PORT = 4000
BUFFER_SIZE = 1024
PATH = "/media/usb/"

s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

def Play():
        isPlaying = False
        player = pyglet.media.Player()
        while 1:
                data = conn.recv(BUFFER_SIZE)
                if not data: break
                elif data == "Pause\r\n":
                        if isPlaying == True:
                                player.pause()
                                isPlaying = False
                        else:
                                player.play()
                                isPlaying = True
                elif data[0] == "P" and data[1] == "l" and data[2] == "a" and data[3] == "y":
                        song = ""
                        i = 6
                        while i < len(data):
                                song += data[i]
                                i += 1
                        player.pause()
                        player = None
                        player = pyglet.media.Player()
                        song = song.split(".wav")
                        playSong = song[0]
                        playSong += ".wav"
                        player.queue(pyglet.media.load(PATH + playSong ,streaming=False))
                        player.play()
                        isPlaying = True

def searchSongs():
       StickInhalt = os.listdir(PATH)
       lieder = []
       for lied in StickInhalt:
                if lied != "System Volume Information" and lied[0] != ".":
                        lieder.insert(len(lieder),lied)
       for lied in lieder:
                conn.send(lied + "|")
       Play()

def Connect():
        s.bind((TCP_IP, TCP_PORT))
        s.listen(1)
        conn, addr = s.accept()

        while True:
                data = conn.recv(BUFFER_SIZE)
                if not data: break
                else:
                        print "Verbunden mit:",addr
                        return conn
conn = Connect()
searchSongs()

conn.close()

V.a. wie ich das "Play" aus dem Stream hole ershciet mir äußerst "unelegant"
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

So ganz schnell in kürze:

- unidiomatische Einrückungen (PEP8 schreibt 4 Spaces vor!)
- ``1`` als Ersatz für ``True``
- Code auf Modulebene
- String Zusammenbau mittels ``+`` (Zeile 40)
- Zeile 29: :shock: echt jetzt? Ist Dir da eigentlich klar, was Du da machst?

Allgemein fehlt die komplette Dokumentation
encoding_kapiert = all(verstehen(lesen(info)) for info in (Leonidas Folien, Blog, Folien & Text inkl. Python3, utf-8 everywhere))
assert encoding_kapiert
jerch
User
Beiträge: 1669
Registriert: Mittwoch 4. März 2009, 14:19

@Quakxi:
Script "schön" genug? - die Frage ist - wofür? Einen Schönheitspreis gewinnst du damit nicht, neben komischer Codestrukturierung hast Du die Annahme drin, dass die "Play/Pause"-Befehle immer bei Index 0 im String stehen. Das ist schlichtweg nicht der Fall, wenn ein Client schnell hintereinander "kommuniziert" oder die Eingabe mal länger als Deine Puffergröße wird (z.B. bei langen Dateinamen). Da ist die Vorgehensweise beim Wort "Play" das geringste Problem.
Falls Du daran interssiert bist, wie man einen TCP-Server robust schreibt, gibts in der Standardbibliothek das Modul socketserver (https://hg.python.org/cpython/file/2.7/ ... tServer.py).

Da Du weniger am Datenaustausch als an Prozeduraufrufen interessiert bist, xmlrpc gibts auch in der Standardbibliothek. Mit dem ThreadinMixin bekommt man das dann auch multithreaded frei Haus.
Sirius3
User
Beiträge: 17741
Registriert: Sonntag 21. Oktober 2012, 17:20

@Quakxi: hier geht es erst mal nicht nur um schön, sondern wie jerch schon geschrieben hat, darum, dass es noch gravierende Fehler gibt. Wie soll denn der Client wissen, dass alle Liedtitel übertragen sind, wenn Du kein Ende-Zeichen mitschickst?
Zur Schönheit: Es gibt globale Variablen und Code auf Modulebene. Zeile 30 bis 34 wären ein kurzer Einzeiler. Das ist das Antipattern eines Antipatterns (for-Schleife über range(len(x)) als while-Schleife geschrieben). Explizites prüfen auf True ist unnötig. Was bewirkt Zeile 36?
Antworten