Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
data = ['HTTP/1.1 200 OK', 'CACHE-CONTROL: max-age=1810', 'DATE: Wed, 14 May 2014 12:15:19 GMT', 'EXT:', 'LOCATION: http://192.168.94.57:9000/DeviceDescription.xml', 'SERVER: Windows NT/5.0, UPnP/1.0, pvConnect UPnP SDK/1.0', 'ST: uuid:7076436f-6e65-1063-8074-78542e239ff5', 'USN: uuid:7076436f-6e65-1063-8074-78542e239ff5', 'Content-Length: 0', '', '']

From the above list, I have to extract the .xml link.

My code:

for element in data:
    if 'LOCATION' in element:
        xmllink = element.split(': ').[1]

It's taking too much time. How can I make this faster?

Actually I am doing SSDP discovery for finding devices in a network. After sending the M-SEARCH command, devices send a datagram packet which I have taken in a data variable. From this I have to extract the file link of that device for processing it.

When I use indexing to extract, it was done quickly.

share|improve this question
4  
I cannot understand how a split and a small array like that can take too much time. Have you use some sort of profiling to make sure that this part is the problem? –  Marc-Andre May 14 at 14:07
    
@Marc-Andre actually i am doing ssdp dicovery for devices in network, after sending M-SEARCH command devices respond with a datagram packet which i have taken in "data" and its taking so much time to process this, earlier i have used direct indexing to find "LOCATION" and it was done quickly –  Patrick May 14 at 14:15
2  
those information are important for a review! You should edit the question. –  Marc-Andre May 14 at 14:27

3 Answers 3

You want to test for element.startswith('LOCATION: '). You are doing 'LOCATION' in element, which is not only slower since it has to check at every position of every element, it might also lead to false matches.

Also, element and data are poor names. I suggest header and headers, respectively.

My suggestion:

LOC = 'LOCATION: '
xmllinks = [header[len(LOC):] for header in headers if header.startswith(LOC)]
if xmllinks:
    xmllink = xmllinks[0]
share|improve this answer
1  
Is it safe to assume there will be a single space after 'LOCATION:' based on his original implementation? Searching for simply 'LOCATION:' then using strip() to remove any additional white space would probably be more secure. –  BeetDemGuise May 14 at 15:15

I am not sure what is causing problem. This piece of code should not take up a lot of time. But here are some of the suggestions to speed up your code:

  1. Instead of list, create a set. The lookup time in set is constant.
  2. But the main problem with set is uses up more memory in comparison to list. So another option is to keep a sorted list (if possible) and use the bisect module to search the list.

Now some style suggestions:

for element in data:
  if 'LOCATION' in element:
    xmllink = element.split(': ').[1]

rewrite it as

for element in data:
   if 'LOCATION' in element and ':' in element:
     xmllink = element.split(':')[1].strip()

This ensures that if a string like 'LOCATION something' is in list then it will not raise any errors.

The .strip() is a better way to remove trailing whitespaces

share|improve this answer
    
Using a set only works if you want to check the whole element, set.contains("LOCATION: http://www.example.com/test.xml") would be fast, but what is needed here is something like set.startsWith("LOCATION: "), which doesn't exist. –  Simon André Forsberg May 14 at 16:10

With such a small input it should be lightning fast. Anyway, shouldn't you break after finding the element? I'd write:

xmllink = next(s.split(":", 1)[1].strip() for s in data if s.startswith("LOCATION:")
share|improve this answer
    
how does this code ensures speedup ? –  Pranav Raj May 14 at 14:26
1  
because it breaks after finding the match. Not sure how the OP is having speed problems here, though –  tokland May 14 at 14:26
1  
@tokland Read the comments under the question, he's giving a bit more information about the speed problem. –  Marc-Andre May 14 at 14:28
    
While I agree with the break suggestion, I think using regex is totally overkill. –  Simon André Forsberg May 14 at 16:12
    
some refactors. –  tokland May 14 at 20:14

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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