HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Optimization of "def do_GET(self)"

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
optimizationdefdo_getself

Problem

This code works nicely for my purposes, but I would like to improve it exponentially.

What are some steps I can take? I'm interested in optimization, refactoring, and clean-code.

def do_GET(self):
    qs = {}
    path = self.path
    print path
    if path.endswith('robots.txt'):
      self.send_response(500)
      self.send_header('Connection', 'close')
      return
    if '?' in path:
      path, tmp = path.split('?', 1)
      qs = urlparse.parse_qs(tmp)
      print path, qs
    if 'youtube' in path:
     ID = qs.get('vID')
     LTV = qs.get('channelID')
     EMAIL = qs.get('uEmail')
     TIME = qs.get('Time')
     userID = qs.get('uID')
     URL = qs.get('contentUrl')
     channelID = str(LTV)[2:-2]
     print channelID
     uEmail = str(EMAIL)[2:-2]
     print uEmail
     ytID = str(ID)[2:-2]
     print ytID
     ytURL = str("https://www.youtube.com/tv#/watch?v=%s" % ytID)
     print ytURL
     uTime = str(TIME)[2:-2]
     print uTime
     uID = str(userID)[2:-2]
     print uID
     contentUrl = str(URL)[2:-2]
     print contentUrl
     if sys.platform == "darwin":
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./test.py %s'%ytURL, shell=True)
     elif sys.platform == "linux2":
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./test.py %s'% ytURL, shell=True)
    #subprocess.Popen('chromium-browser https://www.youtube.com/tv#/watch?v=%s'% ytID, shell=True)
      sleep(2)
    #subprocess.Popen("./fullscreen", shell=True)
     elif sys.platform == "win32":
      os.system('tskill chrome')
      browser = webbrowser.open('http://youtube.com/watch?v="%s"'% ytID)
      log.warn("'%s':,video request:'%s' @ %s"% (channelID, ytID, time))
    #playyt = subprocess.Popen("/Users/Shared/ltv.ap

Solution

How about using

IS, LTV, EMAIL, TIME, userID, URL  = map(\
    qs.get, ['vID', 'channelID', 'uEmail', 'Time', 'uID', 'contentUrl'])

channelID, uEmail, ytID, uTime, uID, contentUrl = map(\
    lambda x : str(x)[2:-2], [LTV, EMAIL, ID, TIME, userID, URL])

for i in (channelID, uEmail, ytID, uTime, uID, contentUrl):
    print i

ytURL = str("https://www.youtube.com/tv#/watch?v=%s" % ytID)
print ytURL


instead of

ID = qs.get('vID')
LTV = qs.get('channelID')
EMAIL = qs.get('uEmail')
TIME = qs.get('Time')
userID = qs.get('uID')
URL = qs.get('contentUrl')
channelID = str(LTV)[2:-2]
print channelID
uEmail = str(EMAIL)[2:-2]
print uEmail
ytID = str(ID)[2:-2]
print ytID
ytURL = str("https://www.youtube.com/tv#/watch?v=%s" % ytID)
print ytURL
uTime = str(TIME)[2:-2]
print uTime
uID = str(userID)[2:-2]
print uID
contentUrl = str(URL)[2:-2]
print contentUrl


The first one just prints ytURL at the end but it can be changed easily if you really want. Other than that they do the same thing. But I am sure the first one is much easier to modify and maintain.

Other than that I can only say after I know what this program is about.

It might also be a good idea to keep os-specific functions(like you are doing after checking for the os) into separate functions. It would make maintaining and modifying the code much easier.

You should also read PEP8, particularly the part about code-layout. Your indentation is not good.

What I meant was that this can be converted into

if sys.platform == "darwin":
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./test.py %s'%ytURL, shell=True)
elif sys.platform == "linux2":
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./test.py %s'% ytURL, shell=True)


this equivalent structure

OS_DETAIL = { 'darwin' : 2, 'linux2' : 1}
for i in xrange(OS_DETAIL[sys.platform]):
    subprocess.Popen('./killltv', shell=True)
    sleep(1)
else:
    subprocess.Popen('./test.py %s'% ytURL, shell=True)


It doesn't solve the complete problem because I am not sure whether you'll need those commented statements or not. If the sleep(2) is actually needed in case of linux2 then you can store a tuple in the OS_DETAILS dictionary. But the problem with this approach is your win32 part. It is totally different. It can be taken care of using an structure like this.

if sys.platform in ['darwin', 'linux2`]:
    #place the code I have just suggested for these 2 
elif sys.platform == "win32"
    #place your code for win32


But as you can see this isn't the best way. It is better than the current solution but if the code for win32 can be refactored in some way (I don't know its functionality so don't ask) to fit the same pattern then it all comes down to a simple dictionary and a loop structure.

Code Snippets

IS, LTV, EMAIL, TIME, userID, URL  = map(\
    qs.get, ['vID', 'channelID', 'uEmail', 'Time', 'uID', 'contentUrl'])

channelID, uEmail, ytID, uTime, uID, contentUrl = map(\
    lambda x : str(x)[2:-2], [LTV, EMAIL, ID, TIME, userID, URL])

for i in (channelID, uEmail, ytID, uTime, uID, contentUrl):
    print i

ytURL = str("https://www.youtube.com/tv#/watch?v=%s" % ytID)
print ytURL
ID = qs.get('vID')
LTV = qs.get('channelID')
EMAIL = qs.get('uEmail')
TIME = qs.get('Time')
userID = qs.get('uID')
URL = qs.get('contentUrl')
channelID = str(LTV)[2:-2]
print channelID
uEmail = str(EMAIL)[2:-2]
print uEmail
ytID = str(ID)[2:-2]
print ytID
ytURL = str("https://www.youtube.com/tv#/watch?v=%s" % ytID)
print ytURL
uTime = str(TIME)[2:-2]
print uTime
uID = str(userID)[2:-2]
print uID
contentUrl = str(URL)[2:-2]
print contentUrl
if sys.platform == "darwin":
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./test.py %s'%ytURL, shell=True)
elif sys.platform == "linux2":
      subprocess.Popen('./killltv', shell=True)
      sleep(1)
      subprocess.Popen('./test.py %s'% ytURL, shell=True)
OS_DETAIL = { 'darwin' : 2, 'linux2' : 1}
for i in xrange(OS_DETAIL[sys.platform]):
    subprocess.Popen('./killltv', shell=True)
    sleep(1)
else:
    subprocess.Popen('./test.py %s'% ytURL, shell=True)
if sys.platform in ['darwin', 'linux2`]:
    #place the code I have just suggested for these 2 
elif sys.platform == "win32"
    #place your code for win32

Context

StackExchange Code Review Q#28799, answer score: 5

Revisions (0)

No revisions yet.