You are not logged in.
Pages: 1
Hello everyone!
A fresh ArchLinux forum member here. This is my second post here, and I'm glad that it's not in the newbie corner! (I even managed to solve a problem with my first post)
So, this is my story...
I started learning Python 3 four days ago and I wrote a little command line RSS Reader (~120 lines) using the feedparser module.
The rss reader currently has a menu, and also receives command line arguments (run the script with "-h" or "--help" for more info).
It has functionalities like adding/removing URLs from the feeds and reading those feeds.
What I'm asking from you is if anyone wants and has the time to go trough my code, try the script out and get me any feedback, like what should I do next, add any new features or point out some mistakes that I have made while coding. Criticism is welcomed (just keep in mind that I started learning Python few days ago, so don't be too harsh )!
I know I didn't say very much about the RSS reader, but in fact there isn't too much to say. It's a really simple script, but it would mean a lot to me if you can get me any feedback so I can look into the errors I made and fix them (and learn even more while doing it!)
You can get the code HERE.
Thank you for reading!
Offline
You should not change the value of __file__ (which is the path to the script). you want something like:
feedparser_dir = os.path.join( os.path.dirname(os.path.abspath(__file__)), "feedparser" )
if feedparser_dir not in sys.path:
sys.path.insert(0, feedparser_dir)
The comment for storeToDB() is wrong: you give it a list, not an array (arrays also exist in python but they are a different, albeit related, thing).
loadFromDB() and storeToDB() should probably catch IOError. Also, i would rather let loadFromDB() return None or an empty list when the database doesn't exist yet, rather than doing the exception handling at the call site.
In getRSS();
feed_results = {}
for url in feed_urls:
tmp = feedparser.parse(url)
results_for_url = []
for entry in tmp.entries:
results_for_url.append([entry.updated_parsed, entry.title, entry.link])
feed_results[tmp.feed.title] = results_for_url[0:10]
can be simplified to:
# add 'from collections import defaultdict' at the top of the file
feed_results = defaultdict(list)
for url in feed_urls:
tmp = feedparser.parse(url)
for entry in tmp.entries[:10]:
feed_results[tmp.feed.title].append([entry.updated_parsed, entry.title, entry.link])
[entry.updated_parsed, entry.title, entry.link] could probably be a tuple instead of a list.
Also, this may need more error handling depending on how feedparser.parse() works.
in InvokeMenu(), you do:
while True:
printMenu()
try:
menu_input = int(input("Choose an option [1, 2, 3, 4]: "))
except ValueError:
print("No such option, please try again.")
menu_input = int(input("Choose an option [1, 2, 3, 4]: "))
except KeyboardInterrupt:
sys.exit("Goodbye!")
...
The except ValueError block should use 'continue' rather than call int(input(...)) again (which is bad since a ValueError raised from here won't be handled). not sure that you wan't to call printMenu() every time though, so maybe do it has a second imbricated 'while True:' loop, that is:
while True:
printMenu()
while True:
try:
menu_input = int(input("Choose an option [1, 2, 3, 4]: "))
except ValueError:
print("No such option, please try again.")
continue
except KeyboardInterrupt:
sys.exit("Goodbye!")
break
...
For bonus point, you could also raise ValueError when not 1 <= menu_input <= 4.
Still in InvokeMenu():
cnt = 0
for url in feed_urls:
print("{0}. {1}".format(cnt+1, url))
cnt += 1
Can be simplified to the more pythonic:
for cnt, url in enumerate(feed_urls):
print("{0}. {1}".format(cnt+1, url))
Just after that, you do:
del_choose = int(input("Enter the number of the feed you want to delete:"))
You need to check for ValueError here (and check that the number is in the correct range). Since this is exactly the same thing than for menu_input above, you want to factorize that into a function.
The main entry point to the script should be guarded with 'if __name__ == "__main__":'. I usually use something like this:
def main(rawargs):
... do stuff ...
if __name__ == "__main__":
sys.exit ( main(sys.argv) )
Your git repo seems to have two imbricated copies of feedparser. Also, you should add the '__pycache__' directories to .gitignore (there is also a vim swapfile in there).
Offline
Pages: 1