-5
\$\begingroup\$

I am fairly new to Python coding, although I have been coding on and off for 30ish years. I am sure I am writing Python with too much influence from the other languages, such as REXX, VB, and some Powershell.

Maybe I should use more functions? Please let me know where the following the code is straying from being Pythonic, and if it can be more efficient.

Example 1:

The following code was written to provide a TV guide of sorts of the latest files added to a subdirectory of my stored TV programs.

import os
import fnmatch
import time
import datetime

tvseries=[]
currentshow="none"

thestartdate = raw_input('What Start date are you looking for YYYY-MM-DD ')
theenddate = raw_input('What End date are you looking for YYYY-MM-DD ')


for root, dir, files in os.walk("i:/Video\\tvseries\\"):
    depth=root.count('\\')
    if depth==2:
        throway1, throwaway2, goodstuff = root.split('\\')
        currentshow=goodstuff
        other=">>"
    elif depth==3:
        throwaway1, throwaway2, goodstuff, other = root.split('\\')
    for items in fnmatch.filter(files, "*"):
        thecheck = os.path.join(root,items)
        thecheck2 = os.path.getmtime(thecheck)
        tcreatetime = str(datetime.datetime.fromtimestamp(thecheck2))
        if tcreatetime[0:10] >= thestartdate and tcreatetime[0:10] <= theenddate:
            tvseries.append(tcreatetime[0:10]+"|"+currentshow+"|"+other+"|"+items)
print len(tvseries), " tvseries"
tvseries.sort()
f = open("i:/Video/info/newshows.txt",'w')
f.write('New Shows added between '+thestartdate+' and '+theenddate+'\n\n')
f.write(str(len(tvseries))+' files added during this timespan'+'\n\n')
for item in tvseries:
    f.write(item+'\n\n')
    print item
f.close()
print "file written to disk at i:/Video/info/newshows.txt"

Example 2: client server with sockets

I wanted a client server example I would actually use. The "server" runs on a machine with a directory containing either Python code or some executables like IPSCAN.exe that I often need out and about to diagnose/fix problems. It waits for a connection then pushes a list of files in the directory to the client. If the client asks for it, it sends a file. It uses two different sending algorithms for .exe and everything else. I probably need to use the .exe method for PDFs as well, but I have not tried that yet.

The "client" runs on another machine and receives a directory list. It adds numbers to the list so the user can enter the number to receive the file. The destination directory must already exist, it is not validated by the client program. The client enters the number or the code alldone. If alldone is entered, it is passed to the server, which then shuts down (I should change the server so it just resets the socket and waits for another connection).

Server code:

# TCP Server Code this code was written in Python 2.75
# this is version 1.00 2013-11-11 please match with client versions/date for compatibility.

#If you don't bind to YOUR IP this won't work.  I am doing this with fixed IP
#I should include code here incase you are on dhcp..  That adds
#complication to the client side though... what host does the client connect to?
#change to 127.0.0.1 if you want to test all on one machine
host="192.168.5.9"               
port=4440                   

from socket import *                
import os
import fnmatch
import time
import datetime

s=socket(AF_INET, SOCK_STREAM)

s.setsockopt(SOL_SOCKET, SO_REUSEADDR, 1) 
s.bind((host,port))

s.listen(1)

print "Listening for connections.. "

q,addr=s.accept()

print "Established connection to client at ", addr

data = "nope"

thecmd = "nutin"
while thecmd != "alldone":

    filelist=[]
    for root, dir, files in os.walk("d:/Sendfiles\\"):
        for items in fnmatch.filter(files, "*"):
            thecheck = os.path.join(root,items)
            thecheck2 = os.path.getmtime(thecheck)
            tcreatetime = str(datetime.datetime.fromtimestamp(thecheck2))
            filelist.append(tcreatetime[0:10]+"|"+items)
    print "File List requested by client"
    for items in filelist:
        theitem = items.ljust(100)
        q.send(theitem)

    q.send("Close")
    print "File List sent to client"

    thecmd = q.recv(7)
    thecmd = thecmd.strip(' ')
    if thecmd != "alldone":
        thenum = int(thecmd)
        theentry = filelist[thenum-1]
        thedate, thefile = theentry.split("|")
        theopenfile = "d:/Sendfiles/"+thefile
        if thefile[len(thefile)-3:]=='exe':
            sendfile = "@@XF"+thefile
            sendfile = sendfile.ljust(500)
            q.send(sendfile)
            print "Sending EXE File ... ",thefile
            f = open(theopenfile,'rb')

            while True:
                theline = f.read(500)
                if theline:
                    q.send(theline)
                else:
                    break
        else:
            sendfile = "@@TF"+thefile
            sendfile = sendfile.ljust(500)
            q.send(sendfile)
            print "Sending Text File ... ",thefile
            f=open(theopenfile,'r')
            for line in f:
                theline = line.strip('\n')
                theline = theline.ljust(500)
                q.send(theline)

        f.close()
        print "File Sent"
        q.send("Done")
        q.recv(8)


print "Service Shutdown requested by client "
print "Service shutting down..."
q.close()

Client code:

# TCP Client Code  This script was written with python 2.75
# this is version 1.00 2013-11-11 please match with server versions/date for compatibility :)
from socket import *

#change this to 127.0.0.1 if want to test the client and server on the same machine
#note you need to start two python environmetns to make this work and run the client
#in one and the server in another.  After starting the first one (windows environment
#just hold shift and single left click on the running one to start another :)
host="192.168.5.9"

port=4440

s=socket(AF_INET, SOCK_STREAM)

s.connect((host,port))

thecommand="notdone"

while thecommand != "alldone":

    tfc=1
    msg="Nope"
    while msg<>'Close':

        msg=s.recv(100)
        msg = msg.strip(' ') 
        if msg != "Close":
            print tfc,") ",msg,'\n'

        tfc=tfc+1

    thecommand = raw_input(' Please enter a file # to recieve or "alldone" to end ')
    if thecommand=="":
        thecommand="alldone"
    if thecommand != "alldone":
        sendit = str(thecommand).ljust(7)
        s.send(sendit)

        tgetit="notdone"
        print "Getting file from server "
        xfertype="?"
        while tgetit != "Done":
            tgetit=s.recv(500)
            thechunk = len(tgetit)
            if tgetit[0:4]=="@@XF":
                thefile = tgetit[4:].strip(' ')
                xfertype="E"
                print "Start recieving EXE file ",thefile
                f=open(thefile,'wb')
            elif tgetit[0:4]=="@@TF":
                thefile = tgetit[4:].strip(' ')
                xfertype="T"
                print "Start recieving TXT file ",thefile
                f=open(thefile,'w')
            elif tgetit[0:4]=="Done":
                tgetit = "Done"
                f.close()
                print "\nLast bit of file was length of ",thechunk
                print "\nFinished receiving file ",thefile
                print "\nNote that the file is saved where this python file is executed from"
                dummy=raw_input("Press enter to continue ")
                s.send("Continue")
            else:
                if xfertype=="T":
                    tgetit=tgetit.strip(' ')
                    tgetit=tgetit+'\n'
                if thechunk>0:
                    f.write(tgetit)
    else:
        s.send("alldone")

print "Thanks for using the python file transfer program"
print "\nService now shutting down ...."

s.close()
\$\endgroup\$

closed as too broad by Stephen Rauch, Mast, Sᴀᴍ Onᴇᴌᴀ, Graipher, Dannnno Jun 27 '18 at 21:33

Please edit the question to limit it to a specific problem with enough detail to identify an adequate answer. Avoid asking multiple distinct questions at once. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 8
    \$\begingroup\$ If you have two separate pieces of code to review, I think you should post them as two separate posts. \$\endgroup\$ – svick Nov 11 '13 at 22:42
2
\$\begingroup\$

Where are your functions? I see three large chunks of loops and if/elif/else blocks, but absolutely 0 functions.

Just because python is a scripting language, doesn't mean you should treat it as one. This is a piece of work that you created, not a one-off script that you will throw away afterwards. Put some effort and refactor pieces of it into functions.

I say this because your code is completely unsupportable. It may be readable to you, but as soon as you give it to someone else, they are lost. Bear in mind, that might be you in the future, looking at the code you wrote weeks/months/years before.

\$\endgroup\$
1
\$\begingroup\$

Coding style

Python has an official style guide called PEP8. Please follow it, and the code will become more pythonic, as you wished, and easier to read for everyone.

Naming

In this code:

throway1, throwaway2, goodstuff = root.split('\\')

"goodstuff" is poorly named as it doesn't help the reader understand its purpose.

While it's clear that "throway1" (with a typo) and "throwaway2" are things to be ignored, they indicate a code smell, as throwing things away sounds wasteful. Code smell like this indicates that probably there's a better way of doing it.

Handling multiple depths

Instead of handling the different depths like this:

if depth==2:
    throway1, throwaway2, goodstuff = root.split('\\')
    currentshow=goodstuff
    other=">>"
elif depth==3:
    throwaway1, throwaway2, goodstuff, other = root.split('\\')

I suggest something like this:

parts = root.split('\\')
if depth == 2:
    currentshow = parts[2]
    other = ">>"
elif depth == 3:
    currentshow = parts[2]
    other = parts[3]

Notice that the strange and unused "throwaway1", "throwaway2" just went away.

Working with files

Instead of this:

f = open("i:/Video/info/newshows.txt",'w')
f.write(...)
# ...
f.close()

The recommended, safer way to write is using with:

with open("i:/Video/info/newshows.txt",'w') as f:
    f.write(...)
    # ...

Notice that f.close() is no longer necessary, Python will automatically call it when leaving the scope of with, so it's guaranteed you'll never forget to call it.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.