Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have the following code.

Aim is to guess a four letter string. Each letter can be tried four times. If the letter cannot be guessed, we go to the next letter until the fourth letter is reached.

I looked at my code and even though I'm satisfied because I've done everything by myself, I'm wondering how I can improve it.

I'm probably relying too much on if but so far, I do not have ideas on what I can do to make it leaner.

If you have any tips, they are more than welcomed.

# -*- coding: utf-8 -*-


def typeYourword():
  global yourword
  yourword= raw_input("type your word \n")
  while len(yourword)<4 or len(yourword)>4:
    yourword= raw_input("type your word again \n")
  else:
    print("Now your word : ",yourword," has the correct format")
    return yourword



#yourword='tata'


def monMot(tonmot):
    global var_glob
    var_glob=list(tonmot)
    return var_glob


def comptage(monmot):
    global var_comptage
    var_comptage = 0
    for cpte in range(0,(len(monmot))):
      print("this is your " + str(cpte+1) + " letter")
      for count in range(1,5):
        comptage_mot=cpte+1
        b = raw_input("Type your letter \n")
        while len(b)>1 or len(b)==0:
          b=raw_input("type your letter again \n")
        else: 
          if var_glob[cpte] == b and comptage_mot<4: 
            var_comptage+=1
            print ("yes")
            break
          elif var_glob[cpte] == b and comptage_mot==4:
            var_comptage+=1
            print ("You won. End of game")
            break
          else:
            if count == 4 and comptage_mot<4:
              print("this was your "+str(count)+ " last try for the "+str(comptage_mot)+" letter. let's go to the next letter")
            elif count == 4 and comptage_mot==4:
              print("this was your last try for the word. You guessed "+str(count)+ " letters. End of game.")
            else:
              print("wrong letter, try again" + " ,this was your " + str(count) + " try. Now your " +str(count+1) + " try")
    print(var_comptage)         



typeYourword()
monMot(yourword) 
comptage(var_glob)
share|improve this question
up vote 3 down vote accepted

Your game is in English, but your function and variable names are partly in French. Consistency is important when writing code, especially if you share it with others around the world (like you did when posting it here). Consider sticking to English for everything.

I'm not saying global is evil, but it has been known to cause problems for people who are still learning the basics of a language. It works, but it's often not the best way. On top of all, it's not necessary if you re-structure your code. The preferred way is using an Object-Oriented approach (read more about here).

Consider placing the body of your script in a main function.

def main():

    typeYourword()
    monMot(yourword) 
    comptage(var_glob)

if __name__ == '__main__':
    main()

This will come in handy if you ever decide to import this code as a module into another script.

I already mentioned your function and variable naming, but there's another thing to consider. When in doubt, it doesn't hurt to follow a style guide. Python has a well-known style guide that is followed by many: PEP8 You can check for some of the style violations (like not leaving enough space between operators and variables) here.

Lines like:

comptage_mot=cpte+1

Turn into:

comptage_mot = cpte + 1

Which are more readable. Python likes spaces, use them. Function names go in snake_case: lower-case with words separated by underscores. You can read all about it in the PEP8, definitely worth a read.

share|improve this answer
    
hi @mast, many thanks for your feedback. One thing I'm wondering is the following : if __name__ == '__main__': main() What does it mean, please? and what do you mean by ` This will come in handy if you ever decide to import this code as a module into another script.` ? Thanks for enlighten me. – Andy K Mar 3 at 18:00
    
@AndyK main in the Python docs. – Mast Mar 3 at 21:49
    
quick one: If I wrote, if __name__ == "__toto__": toto(), would it work? – Andy K Mar 4 at 14:57
    
@AndyK No. __name__ is a special variable. – Mast Mar 4 at 18:59

Firstly, your function names do not conform to standards. For example, typeYourword should be typeYourWord, thought I'd suggest actually using something similar to inputWord, so the verb is related to what the program does rather than the user.

You should read PEP 8, or set your GUI (If you are using one) to warn you of violations of the standard. For example, you are missing a lot of spaces around your operators. For example, b=raw_input("type your letter again \n") should be b = raw_input("type your letter again \n").

Your indent length varies between different functions making the code a bit harder to reason about.

The use of global variables is frowned upon. You are already returning the variable from typeYourword, so consider storing this in a variable and passing it into the other functions. For example:

word = typeYourword()
monmot(word)

monmot appears to have no function, as strings can already be indexed, though perhaps I am not seeing something there.

You do checks for greater than or less than when you could just be using the != operator to check for inequality (while len(yourword) != 4)

Code that is run when the module is loaded directly (the last three lines) should be contained within a check for __name__ == "__main__"

You do not need the else clause on the while loops, this is mainly for complicated loops involving break statements, and you can just outdent your code:

  while len(yourword)<4 or len(yourword)>4:
    yourword= raw_input("type your word again \n")
  print("Now your word : ",yourword," has the correct format")
  return yourword

Hopefully this can help you, and comment if you want anything I've said to be clarified

share|improve this answer
3  
Welcome to Code Review! Good job on your first answer. – SirPython Mar 1 at 18:31
    
Hi @meiamsome what do you mean by Code that is run when the module is loaded directly (the last three lines) should be contained within a check for __name__ == "__main__" ? – Andy K Mar 3 at 18:01
    
@AndyK In python there are two ways for a module to be loaded. Either when it is executed directly, or when another module loads it with an import statement. When it is loaded from another module, that module is presumably looking for this module's functions/classes/etc. It should not automatically run code that has effects (Like taking input from the standard input or printing things out). To prevent this happening you can check the variable __name__, which is the name the module was included with and is set to "__main__" if it was loaded directly. – meiamsome Mar 5 at 19:17
    
Hi @meiamsome, do you have an example by any chance...? – Andy K Mar 5 at 19:30
    
@AndyK Sure thing, there are numerous examples, even within the built in modules. For example: github.com/python/cpython/blob/master/Lib/http/server.py Here, the __name__ check (At the bottom) is used to execute a server for testing (run with python -m http.server from command line). This is certainly not behaviour you would expect if you were to import the module directly in the python code, and so it is wrapped in the protective if statement. – meiamsome Mar 6 at 1:07

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.