patternpythonMinor
Optimization of "def do_GET(self)"
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.
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.apSolution
How about using
instead of
The first one just prints
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
this equivalent structure
It doesn't solve the complete problem because I am not sure whether you'll need those commented statements or not. If the
But as you can see this isn't the best way. It is better than the current solution but if the code for
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 ytURLinstead 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 contentUrlThe 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 win32But 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 ytURLID = 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 contentUrlif 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 win32Context
StackExchange Code Review Q#28799, answer score: 5
Revisions (0)
No revisions yet.