2
\$\begingroup\$

My first real program in python, so probably needs a lot of work. I've been taking a class and it's really slow and I was self taught most over past 2 weeks. Anything I should do differently?

#Programmer: Delirious Mistakes
#Date: October 7, 2015
#File: Advanced Shape Calculator V1

#This file will calculate the measurements of the shapes.


from __future__ import division
import time
import math




#creates a function for the area
def Area_R(num1, num2):
    return (1.0 / 4.0) * num1 * (num2 ** 2.0) * (1.0 / math.tan(math.pi / num1))
    #num1 = number of sides,,, num2 = measure of side

#creates a function for the perimeter
def Perm_R(num1, num2):
    return num1 * num2


def main():
    print "Welcome to my Advance Shape Calculator V1!"
    time.sleep(.7)
    repeat = "d"
    print "Please answer questions using the choices (A, B, C, etc.)"
    time.sleep(2.1738)
    while repeat != "Quit" and repeat != "quit" and repeat != "b" and repeat != "B" and repeat != "no" and repeat != "No":
        print "A) Round Edges"
        print "B) Straight Edges"
        Edges1 = raw_input("Does the shape have round edges or straight edges?: ")
        if Edges1 == "a" or Edges1 == "A" or Edges1 == "Round Edges" or Edges1 == "round edges":
            print "A) Circle"
            print "B) Semi-Circle"
            Circle = raw_input("Is it a circle or semi-circle?: ")
            if Circle == "A" or Circle == "a" or Circle == "Circle" or Circle == "circle":
                radius_C = input("What is the radius (1/2 of the Diameter)?: ")
                Area_C = math.pi * radius_C ** 2.0
                Circum_C = 2.0 * math.pi * radius_C
                Diameter_C = 2.0 * radius_C
                print "The radius is " + str(radius_C) + ". "
                time.sleep(.5)
                print "The diameter is " + str(Diameter_C) + ". "
                time.sleep(.5)
                print "The circumference is " + str(round(Circum_C, 2)) + ". "
                time.sleep(.5)
                print "The area is " + str(round(Area_C, 2)) + ". "
                time.sleep(5)
            elif Circle == "B" or Circle == "b" or Circle == "Semi-Circle" or Circle == "semi-circle":
                radius_S = input("What is the radius (1/2 of the Diameter)?: ")
                Area_S = math.pi * radius_S ** 2.0 * .5
                Diameter_S = 2 * radius_S
                Per_S = ((math.pi * 2 * radius_S) / 2) + Diameter_S
                print "The radius is " + str(radius_S) + ". "
                time.sleep(.5)
                print "The diameter is " + str(Diameter_S) + ". "
                time.sleep(.5)
                print "The perimeter is " + str(round(Per_S, 2)) + ". "
                time.sleep(.5)
                print "The area is " + str(round(Area_S, 2)) + ". "
                time.sleep(5)
            else:
                print "Incorrect input."
        elif Edges1 == "b" or Edges1 == "B" or Edges1 == "Straight Edges" or Edges1== "straight edges":
            sides = input("How many sides does the shape have?: ")
            sideL = input("What is the length of 1 side?: ")
            Area = round(Area_R(sides, sideL), 4)
            Perim = round(Perm_R(sides, sideL), 4)
            print "The area of this figure is: " + str(Area)
            print "The perimeter of the figure is: " + str(Perim)
        else:
            print "Incorrect input."


        time.sleep(4)
        print" "
        print" "
        print "A) yes"
        print "B) No"
        repeat = raw_input("Want to try another?: ")
        time.sleep(1)
main()


#      ___           ___           ___                   ___                       ___           ___           ___     
#     /\  \         /\  \         /\__\      ___        /\  \          ___        /\  \         /\__\         /\  \    
#    /::\  \       /::\  \       /:/  /     /\  \      /::\  \        /\  \      /::\  \       /:/  /        /::\  \   
#   /:/\:\  \     /:/\:\  \     /:/  /      \:\  \    /:/\:\  \       \:\  \    /:/\:\  \     /:/  /        /:/\ \  \  
#  /:/  \:\__\   /::\~\:\  \   /:/  /       /::\__\  /::\~\:\  \      /::\__\  /:/  \:\  \   /:/  /  ___   _\:\~\ \  \ 
# /:/__/ \:|__| /:/\:\ \:\__\ /:/__/     __/:/\/__/ /:/\:\ \:\__\  __/:/\/__/ /:/__/ \:\__\ /:/__/  /\__\ /\ \:\ \ \__\
# \:\  \ /:/  / \:\~\:\ \/__/ \:\  \    /\/:/  /    \/_|::\/:/  / /\/:/  /    \:\  \ /:/  / \:\  \ /:/  / \:\ \:\ \/__/
#  \:\  /:/  /   \:\ \:\__\    \:\  \   \::/__/        |:|::/  /  \::/__/      \:\  /:/  /   \:\  /:/  /   \:\ \:\__\  
#   \:\/:/  /     \:\ \/__/     \:\  \   \:\__\        |:|\/__/    \:\__\       \:\/:/  /     \:\/:/  /     \:\/:/  /  
#    \::/__/       \:\__\        \:\__\   \/__/        |:|  |       \/__/        \::/  /       \::/  /       \::/  /   
#     ~~            \/__/         \/__/                 \|__|                     \/__/         \/__/         \/__/ 
#      ___                       ___           ___           ___           ___           ___           ___     
#     /\__\          ___        /\  \         /\  \         /\  \         /\__\         /\  \         /\  \    
#    /::|  |        /\  \      /::\  \        \:\  \       /::\  \       /:/  /        /::\  \       /::\  \   
#   /:|:|  |        \:\  \    /:/\ \  \        \:\  \     /:/\:\  \     /:/__/        /:/\:\  \     /:/\ \  \  
#  /:/|:|__|__      /::\__\  _\:\~\ \  \       /::\  \   /::\~\:\  \   /::\__\____   /::\~\:\  \   _\:\~\ \  \ 
# /:/ |::::\__\  __/:/\/__/ /\ \:\ \ \__\     /:/\:\__\ /:/\:\ \:\__\ /:/\:::::\__\ /:/\:\ \:\__\ /\ \:\ \ \__\
# \/__/~~/:/  / /\/:/  /    \:\ \:\ \/__/    /:/  \/__/ \/__\:\/:/  / \/_|:|~~|~    \:\~\:\ \/__/ \:\ \:\ \/__/
#       /:/  /  \::/__/      \:\ \:\__\     /:/  /           \::/  /     |:|  |      \:\ \:\__\    \:\ \:\__\  
#      /:/  /    \:\__\       \:\/:/  /     \/__/            /:/  /      |:|  |       \:\ \/__/     \:\/:/  /  
#     /:/  /      \/__/        \::/  /                      /:/  /       |:|  |        \:\__\        \::/  /   
#     \/__/                     \/__/                       \/__/         \|__|         \/__/         \/__/ 
\$\endgroup\$

3 Answers 3

5
\$\begingroup\$

Naming, naming, naming

Real estate has location, programming has naming. Can you, going back to this code in a few months (or even days), tell me what these arguments mean?

def Area_R(num1, num2):

How about this one:

def AreaRegularPolygon(num_sides, side_length):

Yeah, that's pretty dramatic right? Everybody knows what num_sides and side_length means. Nobody knows what num1 is.

Checking lots of conditions

When you find yourself writing:

if Edges1 == "a" or Edges1 == "A" or Edges1 == "Round Edges" or Edges1 == "round edges":

instead try to use the in operator and stick the possible comparators in a tuple:

if Edges1 in ("a", "A", "Round Edges", "round edges",):
    ...

Or even:

if Edges1.lower() in ("a", "round edges",):

Also, going back to naming, what does Edges1 mean? Perhaps:

if edge_choice.lower() in ('a', 'round edges',):

Though why is "round edges" acceptable input? You told the user to select a or b, so make the user do what you tell him!

Functions

Your main is just too long. Break it up into pieces:

if edge_choice.lower() == 'a':
    calculate_circle()
elif edge_choice.lower() == 'b':
    calculate_polygon()
else:
    # log error

Sleeping

Why are you sleeping? Are you trying to make it look like your program is working harder than it is? Or are you doing the sly decrease-sleep-time-to-make-it-look-like-you-improved-performance trick? Either way, drop the sleeps. Although, I was amused the by specificity of sleep(2.1738).

\$\endgroup\$
1
  • \$\begingroup\$ I use the time.sleep function to give the user time to read the statements because I dislike it when I run a program and it floods me with information. Thank you for all the tips it's my first program, so you actually taught me a lot I didn't know. Thanks for your time reviewing it. :) \$\endgroup\$ Commented Oct 9, 2015 at 18:22
4
\$\begingroup\$

One small thing I see that could make it look a little nicer would be changing all of your conditional statements checking if the user's input is upper or lower case with single statements user the .lower() method. For example, your condition

if Edges1 == "a" or Edges1 == "A" or Edges1 == "Round Edges" or Edges1 == "round edges":

Could be stated as:

if Edges1.lower() == "a" or Edges1.lower() == "rounded edges":

I don't think this is much of an efficiency issue; it's just neater and involves less typing :)

\$\endgroup\$
2
\$\begingroup\$

This is a great program. Overall, I would say that it is written as a "script" that reads from top to bottom. There is nothing wrong with this, per se, but if you wanted to up your game I would consider how to use functions (and eventually classes/objects) to make the code more modular. In that way, perhaps, you could separate the control flow of the input gathering and "if" statements, from the math calculations.

For example, if you had a "round_edged_shape()" handler and a "square_edged_shape()" handler it would reduce the main() loop dramatically so that a reader could better tell what was happening at a glance. It would also encapsulate the "round" logic and the "square" logic nicely. Eventually, these feel like classes/objects you could build, but that is probably later in the course.

Some other small style points that I think make the code more readable:

Instead of:

if Edges1 == "a" or Edges1 == "A" or Edges1 == "Round Edges" or Edges1 == "round edges":

When testing a variable against many values, think "in":

if Edges1.lower() in ['a', 'round edges']:

I'm not clear why you have sleep statements at all, I think it would make the interface infuriating. However, but whenever I see a repetitive pattern of statements, I tend to want to write a function. Since you seem to tend to sleep after most prints, consider making that a function:

def print_and_sleep(msg, seconds=0):
    print(msg)
    time.sleep(seconds)

I also prefer string formatting versus the '+' operator because I think it is cleaner and more understandable (maybe because I'm an old "C" programmer, but I think it is also considered good style in Python circles.) Combining that with the above function, your circle printing would look something like:

print_and_sleep("The radius is %0.2f."    % (radius_S), seconds=0.5)
print_and_sleep("The diameter is %0.2f."  % (Diameter_S) , seconds=0.5)
print_and_sleep("The perimeter is %0.2f." % (round(Per_S, 2)), seconds=0.5)
print_and_sleep("The area is %0.2f."      % (round(Area_S, 2)), seconds=5)

Lastly, it is a no-no in Python to just put a function call like main() in the global scope. If your file was "import"ed, it would get executed. The way to avoid this is to always check to see if your program is being executed or imported with the following:

if __name__=="__main__":
    main()

This way, your program will still execute if you "run" it, but not if you "import" it.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the help I'll definitely take the extra functions and string formatting into consideration because of the neatness and less space. I never thought using main() would affect anything seems like I have quite a bit to learn. \$\endgroup\$ Commented Oct 9, 2015 at 18:39

Your Answer

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

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