Here is a simple client for Wwitter that I wrote based on the twitter
library in Python. The aim is to be able to read the feeds from different twitter accounts from the same place in a light way.
I should mention that the code does what I want it to do. Nevertheless, how could I improve it?
(It is available on GitHub.)
#!/usr/bin/python
import sys
import os
import getopt
from twitter import *
# Default values
def_counts = 10
users = []
dir_creds = "./data/oauth/"
# input parameters
def usage():
print "Run: " + sys.argv[0] + " [OPTIONS]"
print "Where OPTIONS is one the following: "
print " -h --help"
print " -c N --counts=N"
print " -u \"name\" --user=\"name\""
print " -a --all"
print " -l --list"
def main(argv):
global def_counts, users
try:
opts, args = getopt.getopt(argv,"hc:u:al",["help","counts=","user=","all","list"])
except getopt.GetoptError:
usage()
exit(2)
for opt,arg in opts:
if opt in ["-h","--help"]:
usage()
exit(1)
elif opt in ["-c","--counts"]:
print "Retrieving "+str(arg)+" tweets."
def_counts = arg
elif opt in ["-u","--user"]:
print "User: "+arg
users.append(arg)
elif opt in ["-a","--all"]:
print "all users"
if os.path.exists(os.path.expanduser(dir_creds)):
token_files = filter(lambda x: x.endswith('.token'), os.listdir(os.path.expanduser(dir_creds)))
for file in token_files:
usr = file[:file.index(".token")]
users.append(usr)
else:
print "No user to be added, path undefined"
exit(2)
elif opt in ["-l","--list"]:
if os.path.exists(os.path.expanduser(dir_creds)):
token_files = filter(lambda x: x.endswith('.token'), os.listdir(os.path.expanduser(dir_creds)))
for file in token_files:
usr = file[:file.index(".token")]
print usr
else:
print "Got the following and I don't know what to do with it:"
print opt + " " + arg
usage()
exit(2)
if __name__ == "__main__":
main(sys.argv[1:])
if len(users) < 1:
users.append("bilbo_pingouin")
col_bgred = '033円[41m'
col_bold = '033円[1m'
col_fgblue = '033円[34m'
col_fggreen = '033円[32m'
col_fggrey = '033円[90m'
col_end = '033円[0m'
for user in users:
print "\n" + col_bgred + col_bold + user + col_end
# Retrieve the credentials for a given account
if not os.path.exists(dir_creds):
os.makedirs(dir_creds) # I just assume there is not race issue here!
file_creds = dir_creds + user + ".token"
MY_TWITTER_CREDS = os.path.expanduser(file_creds)
api_token_file = "data/api_token.dat"
if os.path.exists(api_token_file):
cust_token, cust_secret = read_token_file(os.path.expanduser(api_token_file))
else:
print "ERROR: The app is not identified!"
if not os.path.exists(MY_TWITTER_CREDS):
oauth_dance("Twit on CLI",cust_token,cust_secret,
MY_TWITTER_CREDS)
oauth_token, oauth_secret = read_token_file(MY_TWITTER_CREDS)
# OAuth idnetification
t = Twitter(auth=OAuth(oauth_token,oauth_secret,cust_token,cust_secret))
# Get status
data = t.statuses.home_timeline(count=def_counts)
# Print lines
for c in range(len(data)):
print "* " + col_fggreen+col_bold+data[c]['user']['name']+col_end + ' (' + col_fgblue+data[c]['user']['screen_name']+col_end + ')' + " - " + data[c]['text'] + col_fggrey+" ## "+data[c]['created_at']+col_end
2 Answers 2
For starters using Python 3 or at least being compatible with it would
be a good idea. Meaning that all the print
calls should be using the
function form, i.e. print(...)
.
Next, take a look at PEP8 and use four spaces for indentation.
Parsing command line arguments is a bit nicer with
argparse
, but since
you're already done with it it might not make too much sense to rewrite
that part.
There's duplicated for the "--all"
and "--list"
cases that would be
better served with a common function.
The colour definitions can be taken from some module, e.g. take a look
at termcolor
or
colorama
.
Lastly the for c in range(len(data)):
will always be a pattern that
should be replaced with direct iteration, i.e. for x in data:
. The
line is also way to long to read it easily.
First things first, let's look at argparsing here.
Your current argument parsing code is... a mess. Something like:
import argparse
def main(argv):
global def_counts, users
parser = argparse.ArgumentParser(description='Twitter app')
parser.add_argument('-c', '--counts', help='Count of tweets to retrieve.', required=True)
parser.add_argument('-u', '--user', help='Username', required=True)
parser.add_argument('-a', '--all', help='All Users', required=False)
parser.add_argument('-l', '--list', help='List', required=True)
Allows you much more flexibility in actually reading your arguments. As you might notice this is... much easier to configure than your current parsing. It is also easier for a person such as myself to read.
This allows you do this for a 'help' option if you have at least 1 required argument and get rid of your usage
method:
try:
options = parser.parse_args()
except:
parser.print_help()
sys.exit(0)
If require args are not provided it will throw an exception and print the help (which it will do on -h
or -help
or --foo-nicorn
).
This streamlines your parsing of the args:
def_counts = options['counts']
user = options['user']
users.append(user)
Note that when making strings the following is much more readable:
print "Retrieving {count} tweets.".format(count=def_counts)
print "User: {user}".format(user=user)
Especially when you have longer strings, such as at the end of your code, using format
and the variables as parameters is much more clear.
You might notice the parsing for list
and all
are almost identical, with the exception of printing or appending. I rewrote this, using your options again (note that an unprovided arg to argparse is None
so you can do if arg:
for checking the argument was provided):
if options['all'] or options['list']:
print "all users"
if os.path.exists(os.path.expanduser(dir_creds)):
token_files = filter(lambda x: x.endswith('.token'), os.listdir(os.path.expanduser(dir_creds)))
for f in token_files:
usr = f[:f.index(".token")]
if options['all']:
users.append(usr)
else:
print usr
else:
print "No user to be added, path undefined"
exit(2)
I also changed file
to f
as file
is a keyword in Python.
This removes a fair bit of duplication and also makes reading your logic much less painful.
Everything here should reproduce how your logic parsing your args is currently (full disclaimer: it's Friday night and... no guarantees :-).
For pep8/flake8 you generally want to add spaces after commas, too.