3
\$\begingroup\$

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()
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Welcome to CodeReview.SE ! Can you tell us more about what your code is supposed to do ? \$\endgroup\$
    – SylvainD
    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\$ Jul 29, 2014 at 7:50

3 Answers 3

5
\$\begingroup\$

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]
\$\endgroup\$
1
  • \$\begingroup\$ Thanks Josay! Is there any way so that the values of Dictionary can be random? \$\endgroup\$ Jul 29, 2014 at 8:15
3
\$\begingroup\$

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' ...)}
\$\endgroup\$
3
\$\begingroup\$

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()
\$\endgroup\$

Your Answer

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

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