Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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()
share|improve this question
2  
Welcome to CodeReview.SE ! Can you tell us more about what your code is supposed to do ? –  Josay Jul 29 at 7:45
    
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 –  user3884162 Jul 29 at 7:50

3 Answers 3

up vote 5 down vote accepted

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]
share|improve this answer
    
Thanks Josay! Is there any way so that the values of Dictionary can be random? –  user3884162 Jul 29 at 8:15
1  
Hi! I am not quite sure to understand your answer. In the meantime, I think you should not accept my answer that quickly as it might discourage other people to try to review your code. –  Josay Jul 29 at 8:19
    
ok not a problem, anyway thanks for your answer! –  user3884162 Jul 29 at 8:27

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' ...)}
share|improve this answer

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()
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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