5
\$\begingroup\$

I've made a small program that calculates the cost of flooring for a given area based on the price per sqft and total sqft. I have only been programming for a few days now so I am sure that there are some things I could be doing better.

from decimal import *
import time

print("This program will help you find the cost of floor covering\n")


def program():
    exit_program = ["q", "Q"]

    global move_on
    move_on = ["y", "Y"]

    global more_info
    more_info = ["n", "N"]

    cost_of_tile = Decimal(input("What is the cost of the tile per square foot?"))

    global answer
    answer = input("Do you know how big the room is in sqft? Y/N")

    if answer in move_on:
        global sqft
        sqft = Decimal(input("Please enter the room's size in SQUARE FEET"))
        print("The cost to cover your floor will be $%s" % (sqft * round(cost_of_tile, 2)))
        labor()
        answer = input("Would you like to run the calculator again for another room? Y/N")
        should_i_stay()
    elif answer in more_info:
        answer = input("Do you know the length and the width of the room? Y/N")

        if answer in move_on:
            width = Decimal(input("What is the width of the room?"))
            length = Decimal(input("What is the length of the room?"))
            sqft = width * length
            print("Your room is %s sqft" % sqft)
            print("The cost to cover your room will be $%s" % (sqft * round(cost_of_tile, 2)))
            labor()
        elif answer in more_info:
            print("Sorry we need more information ")

            answer = input("Would you like to run the calculator again? Y/N")
            should_i_stay()
    elif answer in exit_program:
        print("Exiting")
        exit()


def should_i_stay():
    if answer in move_on:
        program()
    else:
        print("Good Bye")
        time.sleep(1)
        exit()


def labor():
    rate = 86
    total_labor = (sqft / 20) * rate
    hours_of_labor = sqft / 20
    if hours_of_labor <= 1:

        hours_of_labor = 1
        total_labor = rate
        print("The labor will be approximately $%s at a rate $%s per "
              "hour for a total of %s hours of labor" % (round(total_labor, 2), rate, hours_of_labor))
    else:
        print("The labor will be approximately $%s at a rate $%s per "
              "hour for a total of %s hours of labor" % (round(total_labor, 2), rate, hours_of_labor))


program()
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to CodeReview.SE! The indentation in your code seems wrong ("line 28; IndentationError: unindent does not match any outer indentation level"). \$\endgroup\$
    – SylvainD
    Feb 5, 2015 at 11:33

1 Answer 1

1
\$\begingroup\$

Separation of concerns

You are mixing user interface and calculations: you should declare function for processing data and doing calculations and other functions to print pretty output. labor() handles both claculating and outputting.

Globals should be avoided

A function should be considered a black box with no knowledge about the outside world, function should never use global variables, because it is hard to mentally keep track of the value of all the global variables. Consider making sqft an argument of labor.

Functions are different from go to

def should_i_stay():
    if answer in move_on:
        program()

So you are basically jumping from one point in the code to an arbitrarly past point in the code. Functions should not be used this way (see above).

User interface

You should strive to offer the best interface possible to the user, your code has several User Interface flaws:

  • There is no space between the question and the space where you must input, this fells awkard.

  • Bad input ends the programme immediately or shows a scary error message:

:

This program will help you find the cost of floor covering

What is the cost of the tile per square foot?about 10 dollars
Traceback (most recent call last):
// Omitted for brevity 
decimal.InvalidOperation: [<class 'decimal.ConversionSyntax'>]

you should learn about exception handling to make the input more reliable.

Best practices

  • You are using a magic number 20 in your labor function. A magic number is a number with no explanation whatsoever why you chose it over another.

  • Do not use from module import *, it will give people (even yourself in the future) problems figuring out where the functions used are taken from and may generate name clashes.

  • Use an if __name__ == '__main__' guard so that you can import your code in other files.

  • Global constants should be declared at the start of the file and be ALL CAPS:

:

EXIT_PROGRAM = ["q", "Q"]

MOVE_ON = ["y", "Y"]

MORE_INFO = ["n", "N"]
\$\endgroup\$

Your Answer

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

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