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

\$\endgroup\$
  • 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\$ – Gameskiller01 May 26 '17 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\$ – Daniel May 26 '17 at 20:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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