1

How would I clean up the following code and make it look better?

I wonder if there is a way to set all these variables and values in an easier and shorter version. What would be an example of doing so? :-)

import feedparser
import memcache
import sys

mc = memcache.Client(["127.0.0.1:11211"])
rss = feedparser.parse('http://example.com/example.rss')
song0 = rss.entries[0].title.encode('latin-1', 'replace').replace("?" , "-")
song1 = rss.entries[1].title.encode('latin-1', 'replace').replace("?" , "-")
song2 = rss.entries[2].title.encode('latin-1', 'replace').replace("?" , "-")
song3 = rss.entries[3].title.encode('latin-1', 'replace').replace("?" , "-")
song4 = rss.entries[4].title.encode('latin-1', 'replace').replace("?" , "-")
song5 = rss.entries[5].title.encode('latin-1', 'replace').replace("?" , "-")
song6 = rss.entries[6].title.encode('latin-1', 'replace').replace("?" , "-")
song7 = rss.entries[7].title.encode('latin-1', 'replace').replace("?" , "-")
song8 = rss.entries[8].title.encode('latin-1', 'replace').replace("?" , "-")
song9 = rss.entries[9].title.encode('latin-1', 'replace').replace("?" , "-")

mc.set("track0", song0);
mc.set("track1", song1);
mc.set("track2", song2);
mc.set("track3", song3);
mc.set("track4", song4);
mc.set("track5", song5);
mc.set("track6", song6);
mc.set("track7", song7);
mc.set("track8", song8);
mc.set("track9", song9);
2
  • This is not really deserving of a question so don't be too surprised if it is closed. However, rss must be an iterable so for item in iterable: do_something it could be done in one line with a list comprehension Commented Mar 5, 2014 at 18:59
  • What I mainly was looking for and failed was the function list. Commented Mar 5, 2014 at 19:48

4 Answers 4

2

You have 10 variables with similar names only differing by a number. It's a sure sign that what you really want to use is a list:

songs = [i.title.encode("latin-1", "replace").replace("?" , "-") for i in rss.entries]
Sign up to request clarification or add additional context in comments.

Comments

2

The cleanest (thus best) way:

import feedparser
import memcache
import sys

mc = memcache.Client(["127.0.0.1:11211"])
rss = feedparser.parse('http://example.com/example.rss')
songs = [] # Initialize “songs” to the empty list
for i in range(0, 10):
    # Add items in “songs”
    songs.append(rss.entries[i].title.encode('latin-1', 'replace').replace("?" , "-"))

for (i, song) in enumerate(songs): # This is equivalent to “for i in range(0, len(songs)+1):” and “song = songs[i]”
    mc.set("track%i" % i, song);

If you really want to keep songs in different variables (I'm almost sure you don't actually want to; more over it is not recommended to use it (see the comments on this question)):

import feedparser
import memcache
import sys

mc = memcache.Client(["127.0.0.1:11211"])
rss = feedparser.parse('http://example.com/example.rss')
for i in range(0, 10):
    locals()['song%i' % i] = rss.entries[0].title.encode('latin-1', 'replace').replace("?" , "-")

for i in range(0, 10):
    mc.set("track%i" % i, locals()['song%i' % i]);

6 Comments

have you actually tried the second solution (using locals())? i'm pretty sure that doesn't work.
It works for me on a small example (locals()['spam'] = 'egg' and print(locals()['spam']))
please look at this: stackoverflow.com/questions/10488296/… even if it works, the doc explicitly warns against it as unsafe.
note that the way you have used locals(), it could be any dictionary - wouldn't specifically have to be the locals, since you get the value out also by dereferencing locals() in your example. in fact, that's how it works - if you try to access the variable song1 directly, it won't work - it's not really added to the local namespace. but locals() returns the same dictionary every time, so you see the addition later.
I just tested, accessing spam after doing locals()['spam'] = 'egg') works. (But I agree the doc says we should not rely on it)
|
1
import feedparser
import memcache
import sys

mc = memcache.Client(["127.0.0.1:11211"])
rss = feedparser.parse('http://example.com/example.rss')
song=[]
for i in range(0,10):
    song[i] = rss.entries[i].title.encode('latin-1', 'replace').replace("?" , "-")
    mc.set("track"+str(i), song[i]);

Comments

0

Using enumerate() with rss.entries would free you from the burden of manually maintaining an index. Best of all, it'd work regardless of the length of rss.entries.

import feedparser
import memcache
import sys

mc = memcache.Client(["127.0.0.1:11211"])
rss = feedparser.parse('http://example.com/example.rss')

song = []
for i, entry in enumerate(rss.entries):
    title = entry.title.encode('latin-1', 'replace').replace("?" , "-")
    song.append(title)
    mc.set("track%d" % i, title)

2 Comments

I had to initialize the list with song = {} otherwise I get IndexError: list index out of range
You ended up using dictionary, not list ({} stands for dictionary literal). Updated the code to work correctly for list.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.