3
\$\begingroup\$

I have some code that asks for a user input, and then uses TTS to convert that into speech and play the file. It then asks the user whether they want to repeat it and convert another input into speech, or whether they want to exit the program. Are there any improvements that I could make to the code?

import webbrowser
import os
import time
import sys
import getpass
from gtts import gTTS
from mutagen.mp3 import MP3
my_file = "C:/Users/%USERNAME%/Desktop/TTS/bob.mp3" #Sets a variable for the file path.
username = getpass.getuser() #Gets the username of the current user.
def repeat():
 while True:
 if os.path.isfile(my_file): #Checks to see whether there is a file present and, if so, removes it.
 os.remove(my_file)
 else:
 None
 tts = gTTS(text = input("Hello there " + username + """. This program is
used to output the user's input as speech.
Please input something for the program to say: """)) #Takes the user's input and uses it for the Text-To-Speech
 tts.save('bob.mp3') #Saves a .mp3 file of the user's input as speech.
 webbrowser.open(my_file) #Opens the .mp3 file
 audio = MP3(my_file) #Sets a variable so that the Mutagen module knows what file it's working with.
 audio_length = audio.info.length #Sets a variable of the length of the .mp3 file.
 time.sleep((audio_length) + 0.25) #Waits until the file has finished playing.
 os.system('TASKKILL /F /IM wmplayer.exe') #Closes Windows Media Player.
 time.sleep(0.5) #Waits until Windows Media Player has closed.
 while True:
 answer = input("Do you want to repeat? (Y/N) ")
 if answer == "y" or answer == "Y":
 return repeat() #Goes back to the beginning of the function if the user wants to try again.
 elif answer == "n" or answer == "N":
 if os.path.isfile(my_file): #Checks to see whether there is a file present and, if so, removes it.
 os.remove(my_file)
 else:
 None
 sys.exit() #Exits the program.
 else:
 print("Sorry, I didn't understand that. Please try again with either Y or N.")
 continue #Goes back to the beginning of the while loop.
repeat() #Calls the function.
200_success
146k22 gold badges190 silver badges479 bronze badges
asked May 26, 2017 at 19:05
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

DRY

Apply the DRY (Do not Repeat Yourself) method. For instance, the check to remove the file. Since it's being called in two places, we can make this a function. I've re-written your code in a more functional style. This is also easier to read what's occurring.

else

Your else: None statements are not doing anything.

continue

Same goes for this in the while loop. This happens anyway.

if

When you check the user's answer, just assume it's going to be varying. Instead of checking for an UPPERCASE 'y' and lowercase, just lower it. Also stripping it to make sure to remove erroneous spaces.

answer = input("Do you want to repeat? (Y/N) ").strip().lower()
if answer in ['yes', 'y']:
# instead of
answer = input("Do you want to repeat? (Y/N): ")
if answer == "y" or answer == "Y":

Exceptions

Since you're interacting with the user from the interpreter, you can catch if the user exits the program and run any cleanup code before cleanly exiting. I've wrapped the loop of the interaction in a try/except statement so that this can be caught.

if name == 'main':

Check this SO post out.

Rewritten

import webbrowser
import os
import time
import getpass
from gtts import gTTS
from mutagen.mp3 import MP3
my_file = "C:/Users/%USERNAME%/Desktop/TTS/bob.mp3" #Sets a variable for the file path.
username = getpass.getuser() #Gets the username of the current user.
def remove_file():
 """Checks if myfile exists and if so, deletes."""
 if os.path.isfile(my_file):
 os.remove(my_file)
def play_tts():
 webbrowser.open(my_file) # Opens the .mp3 file
 audio = MP3(my_file) # Sets a variable so that the Mutagen module knows what file it's working with.
 audio_length = audio.info.length # Sets a variable of the length of the .mp3 file.
 time.sleep(audio_length + 0.25) # Waits until the file has finished playing.
 os.system('TASKKILL /F /IM wmplayer.exe') # Closes Windows Media Player.
 time.sleep(0.5) # Waits until Windows Media Player has closed.
def ask_and_play():
 # Takes the user's input and uses it for the Text-To-Speech
 tts = gTTS(text=input(
 "Hello there " + username + ". This program isn\n"
 "used to output the user's input as speech.\n"
 "Please input something for the program to say: ")
 )
 tts.save('bob.mp3') # Saves a .mp3 file of the user's input as speech.
 play_tts()
def check_continue():
 """Checks if the user wants to continue.
 Returns a boolean value."""
 while True:
 answer = input("Do you want to repeat? (Y/N) ").strip().lower()
 if answer in ['yes', 'y']:
 return True
 elif answer in ['no', 'n']:
 return False
 else:
 print("Sorry, I didn't understand that. Please try again with either Y or N.")
def repeat():
 """Repeatedly ask the user to type text to play,
 and check if the user wants to repeat or exit."""
 while True:
 remove_file()
 ask_and_play()
 if not check_continue():
 raise KeyboardInterrupt
if __name__ == '__main__':
 try:
 repeat() #Calls the function.
 except KeyboardInterrupt:
 # clean up
 remove_file()
 print('Goodbye!')
answered May 26, 2017 at 20:44
\$\endgroup\$
2
\$\begingroup\$

Comments

A core rule of comments is that they add to code, by explaining why you're doing something the way you are. A bad way to use comments is by just repeating what the code does. For example:

my_file = "C:/Users/%USERNAME%/Desktop/TTS/bob.mp3" #Sets a variable for the file path.

The comment is useless. A better choice in this case, would be:

file_path = "C:/Users/%USERNAME%/Desktop/TTS/bob.mp3" 

Without a comment. Another example:

os.path.isfile(my_file): #Checks to see whether there is a file present and, if so, removes it.

the names isfile() and remove() are pretty good indications of what the functions do, so a comment is definitely not needed in that case.

If-else

else:
 None

Isn't good practice. Much better would be to use

else:
 pass

Or even better, leave the else statement out.

Bad variable names

Names like repeat() are not good for functions, they don't explain what happens. A name like text_to_speech() would be more descriptive.

Improved code

Here's my version of your code:

import webbrowser
import os
import time
import getpass
from gtts import gTTS
from mutagen.mp3 import MP3
FILE_PATH = "C:/Users/%USERNAME%/Desktop/TTS/bob.mp3" 
username = getpass.getuser()
def text_to_speech():
 while True:
 if os.path.isfile(my_file):
 os.remove(my_file)
 tts = gTTS(text = input("Hello there " + username + """. This program is
 used to output the user's input as speech.
 Please input something for the program to say: """)) 
 tts.save('bob.mp3') 
 webbrowser.open(my_file)
 audio = MP3(my_file)
 audio_length = audio.info.length
 time.sleep((audio_length) + 0.25) # Wait until the mp3 file has finished
 os.system('TASKKILL /F /IM wmplayer.exe') 
 time.sleep(0.5) 
 while True:
 answer = input("Do you want to repeat? (Y/N): ")
 if answer == "y" or answer == "Y":
 repeat() 
 elif answer == "n" or answer == "N":
 if os.path.isfile(my_file): 
 os.remove(my_file)
 exit()
 else:
 print("Sorry, I didn't understand that. Please try again with Y or N.")
text_to_speech() 

Some changes I made:

  • Removed unnecessary comments

  • Removed sys import, replaced sys.exit() with exit()

  • Added whitespace to make code easier on the eyes

  • repeat() is now called text_to_speech()

Toby Speight
87.6k14 gold badges104 silver badges325 bronze badges
answered May 26, 2017 at 20:21
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks! I've taken all of your suggestions and implemented them into the code. The only thing I didn't change is sys.exit() to exit(); exit() asks for confirmation before closing the program (as the program is still running) whereas sys.exit() does not. Also, in that code you've left the call to the function as repeat() instead of text_to_speech(). Just thought I'd point that out. :) \$\endgroup\$ Commented May 26, 2017 at 20:46
  • \$\begingroup\$ Your'e welcome. If you can, be sure to implement double_j's main() function and call main() for good practice. Also, if you want to use sys.exit(), instead of importing all of sys, use -from sys import exit- \$\endgroup\$ Commented May 26, 2017 at 20:48

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.