# Optimization of “def do_GET(self)”

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)
return
if '?' in path:
path, tmp = path.split('?', 1)
qs = urlparse.parse_qs(tmp)
print path, qs
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
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)
sleep(2)
#subprocess.Popen("./fullscreen", shell=True)
elif sys.platform == "win32":
os.system('tskill chrome')
log.warn("'%s':,video request:'%s' @ %s"% (channelID, ytID, time))
#playyt = subprocess.Popen("/Users/Shared/ltv.ap

• Some explanation of its purpose would be helpful. That way we don't accidentally break it. That is the first feedback. Always use docstring at the beginning of a function explaining its purpose. – Aseem Bansal Jul 22 '13 at 5:00
• Why are you running ./killtv a bunch of times and then running another python script (test.py) from your python script? – raylu Jul 26 '13 at 2:06
• http get request opens a pyside window (test.py), killtv just closes the previous request/window – sirvon Jul 26 '13 at 2:25
• About your os specific code why are you only opening the browser when sys.platform is win32? If you add certain details about what you are trying to do by test.py then the os specific code can be converted into loops based on values stored in dictionaries with key being sys.platform. – Aseem Bansal Jul 30 '13 at 13:12
• @AseemBansal can you elaborate... I think this is something i'm searching for. test.py opens up a pyside window with a url passed in from the get request. – sirvon Aug 3 '13 at 1:32

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

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
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.

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"


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.

• Use 4 spaces for indentation.
• Local variables should use the underscore style: name_of_variable.
• This function is too long. Split code in functions.
• When writing conditionals, try to keep a if/elif/else layout, without early returns, you'll see that readability is greatly enhanced.
• Use dictionaries to hold multiple named values, not one variable per value, that's too messy.
• Some patterns are repeated all over the place, abstract them.
• Don't update variables in-place unless there is a good reason for it (99% of the time there isn't).

I'd write (show_video has been left out, it's clearly another function's task):

def do_GET(self):
url = urlparse.urlparse(self.path)
`