I have accomplished what I wanted to do but I think this can be done better. I want my code to look professional.
What my code does is ask for your name and gives output in below form.
Input: Jack
Output: j = joyful a = awesome c = curious k = kindhearted
Please show me how to code in a professional manner by reviewing the below code.
#!/usr/bin/env python
import sys
import string
names = {
'a': 'awesome',
'b': 'bold',
'c': 'curious',
'd':'delightful',
'e':'emotional',
'f':'fearless',
'g':'gifted',
'h':'helpful',
'i':'imaginary',
'j':'joyful',
'k':'kindhearted',
'l':'lovable',
'm':'memorable',
'n':'naughty',
'o':'open',
'p':'playful',
'q':'quarrelsome',
'r':'reliable',
's':'serious',
't':'thoughtful',
'u':'unique',
'v':'victorious',
'w':'wise',
'x':'xerox copy',
'y':'yummy',
'z':'zealous'
}
def main():
i=j=0
user = raw_input("What is your name?: ")
print ""
while i<len(user):
while j<len(names):
if user[i]==names.keys()[j]:
print " " + user[i] + ' = ' + names.values()[j]
j=j+1
while j!=0:
j=0
i=i+1
if __name__ == '__main__':
main()
-
2\$\begingroup\$ Welcome to CodeReview.SE ! Can you tell us more about what your code is supposed to do ? \$\endgroup\$SylvainD– SylvainD2014年07月29日 07:45:13 +00:00Commented Jul 29, 2014 at 7:45
-
\$\begingroup\$ Yes, I write this just to practice. What it does is ask for your name and gives output in below form. For example if you input "jack", the output will be: j = joyful a = awesome c = curious k = kindhearted \$\endgroup\$user3884162– user38841622014年07月29日 07:50:15 +00:00Commented Jul 29, 2014 at 7:50
3 Answers 3
Style
A few things can be easily detected/improved in your code regarding the style. If you want, you'll find various tools to perform checks automatically : pep8
, pyflakes
, pychecker
. They'll give you a great deal of interesting information, from imported module xxx is not used
, to bad indentation
via missing documentation
, wrong spacing
and invalid constant name
. Also, it might be good for you to know that in Python, there is a style guide called PEP8 and it's definitely worth a read.
After fixing everything to make the tools happy, your code looks like:
"""Module docstring"""
NAMES = {
'a': 'awesome',
'b': 'bold',
'c': 'curious',
'd': 'delightful',
'e': 'emotional',
'f': 'fearless',
'g': 'gifted',
'h': 'helpful',
'i': 'imaginary',
'j': 'joyful',
'k': 'kindhearted',
'l': 'lovable',
'm': 'memorable',
'n': 'naughty',
'o': 'open',
'p': 'playful',
'q': 'quarrelsome',
'r': 'reliable',
's': 'serious',
't': 'thoughtful',
'u': 'unique',
'v': 'victorious',
'w': 'wise',
'x': 'xerox copy',
'y': 'yummy',
'z': 'zealous'
}
def main():
"""Main function"""
i = j = 0
user = raw_input("What is your name?: ")
print ""
while i < len(user):
while j < len(NAMES):
if user[i] == NAMES.keys()[j]:
print " " + user[i] + ' = ' + NAMES.values()[j]
j = j + 1
while j != 0:
j = 0
i = i + 1
if __name__ == '__main__':
main()
Loops
Your are going through your containers (user
and names
) with a while
loop. This is a bit impractical but if you really want to do so, it is clearer to initialise the variable you are going to loop with before the loop so that you not have to try to reset it after the loop.
i = 0
while i < len(user):
j = 0
while j < len(NAMES):
if user[i] == NAMES.keys()[j]:
print " " + user[i] + ' = ' + NAMES.values()[j]
j = j + 1
i = i + 1
Even better, the range
function can generate the values your are looking for so that you do not need to do the incrementation yourself :
for i in range(len(user)):
for j in range(len(NAMES)):
if user[i] == NAMES.keys()[j]:
print " " + user[i] + ' = ' + NAMES.values()[j]
but Python provides you an even better way to iterate over containers :
for c in user:
for j in range(len(NAMES)):
if c == NAMES.keys()[j]:
print " " + c + ' = ' + NAMES.values()[j]
Also, because you are using a dictionary, there is no need to loop over NAMES
. You could lookup keys in the dict naturally:
for c in user:
if c in NAMES:
print " " + c + ' = ' + NAMES[c]
-
\$\begingroup\$ Thanks Josay! Is there any way so that the values of Dictionary can be random? \$\endgroup\$user3884162– user38841622014年07月29日 08:15:15 +00:00Commented Jul 29, 2014 at 8:15
Your initial dictionary could easily be built by python rather than having to type out all the initial letter-word combinations:
NAMES = {word[0]: word for word in ('awesome', 'bold', 'curious' ...)}
Your code looks like a Python program written by a C programmer. That's OK — it takes practice to learn how to accomplish tasks idiomatically in Python.
Unexpected output
It helps to think about ways in which an adversary can break your program. For example:
What is your name?: John Smith
o = open
h = helpful
n = naughty
m = memorable
i = imaginary
t = thoughtful
h = helpful
Even if you don't know the solution, adding a # TODO: handle capital letters, spaces, etc.
comment would be a good idea.
Naming
names
isn't an appropriate name for the the dictionary. I suggest ATTRIBUTES
, as that describes the kinds of values that it contains. Use all-caps to indicate that it should be treated as a constant.
Stray imports
import sys
and import string
are both unnecessary, so you should remove them.
Suggested solution
#!/usr/bin/env python
ATTRIBUTES = {
'a': 'awesome',
'b': 'bold',
'c': 'curious',
# etc...
'z': 'zealous',
}
def main():
user = raw_input('What is your name?: ')
print
for c in user.lower():
attribute = ATTRIBUTES.get(c)
if attribute:
print(' %c = %s' % (c, attribute))
else:
print(' %c' % (c))
if __name__ == '__main__':
main()