5
\$\begingroup\$

The program is meant to collect from the user:

  1. The function under which to calculate the area
  2. The left and right boundaries of the region
  3. The amount and position of rectangles to use to approximate the area

Then run the calculation. Finally, the user is given the option to do another calculation.

import re

print('This is a calculator that determines the Riemann Sum\n\tfor a finite number of rectangles.\n\nYou can quit the program at any time by entering "end".\n')

# Function to escape the program
def escape():
    print('Thanks for using the Riemann Sum Calculator! Goodbye!')
    quit()

# Calculations for the area under the curve
def right_riemann_sum():
    total_area = 0
    for k in range(rectangles):
        x = left_bound + (k + 1) * delta_x
        total_area += delta_x * eval(function.replace("x", str(x)))
    print('The area under the curve is %s' % total_area)

def left_riemann_sum():
    total_area = 0
    for k in range(rectangles):
        x = left_bound + k * delta_x
        total_area += delta_x * eval(function.replace("x", str(x)))
    print('The area under the curve is %s' % total_area)

def midpoint_riemann_sum():
    total_area = 0
    for k in range(rectangles):
        x = left_bound + ((k + 1) - (delta_x / 2))
        total_area += delta_x * eval(function.replace("x", str(x)))
    print('The area under the curve is %s' % total_area)

while True:
    total_area = 0

    function = input("What is the function? (must be entered in python math syntax for now... sorry! \n\nf(x) = " )
    if function.upper() == "END":
        escape()

    left_bound = input("What is the left bound? ")
    if left_bound.upper() == "END":
        escape()
    #Regex number check
    while re.match(r'(\A\-?\d*\.?\d+$)', left_bound) == None:
        if left_bound.upper() == "END":
            escape()
        print("Please enter only numbers.")
        left_bound = input("What is the left bound? ")
    left_bound = float(left_bound)

    right_bound = input("What is the right bound? ")
    if right_bound.upper() == "END":
        escape()
    while re.match(r'(\A\-?\d*\.?\d+$)', right_bound) == None:
        if right_bound.upper() == "END":
            escape()
        print("Please enter only numbers.")
        right_bound = input("What is the right bound? ")
    right_bound = float(right_bound)

    rectangles = input("How many rectangles do you need? ")
    if rectangles.upper() == "END":
        escape()
    while rectangles == "0":
        if rectangles.upper() == "END":
            escape()
        print('The number of rectangles cannot be 0.')
        rectangles = input("How many rectangles do you need? ")
    while re.match(r'(\A\d+$)', rectangles) == None:
        if rectangles.upper() == "END":
            escape()
        print("Please enter only whole numbers.")
        rectangles = input("How many rectangles do you need? ")
    rectangles = int(rectangles)

    delta_x = (right_bound - left_bound) / rectangles

    # Loop if user wants to use same function
    while 1 == 1:
        total_area = 0
        sum_type = input("Do you need the right, left, or midpoint Riemann sum? ")

        # Forcing user to use desired inputs
        while re.match(r'(LEFT|RIGHT|MIDPOINT|END)', sum_type.upper()) == None:
            print("Just use one of the words, man.")
            sum_type = input("Left, right, or midpoint? ")
        sum_type = sum_type.upper()

        if sum_type == "END":
            escape()
        if sum_type == "RIGHT":
            right_riemann_sum()
        if sum_type == "LEFT":
            left_riemann_sum()
        if sum_type == "MIDPOINT":
            midpoint_riemann_sum()

        another = input("Should we do another? ")
        if another.upper() == "END":
            escape()

        while re.match(r'(YES|NO)', another.upper()) == None:
            if another.upper() == "END":
                escape()
            print('Please enter only "yes" or "no"')
            another = input("Should we do another? ")

        # If user doesn't want to do another, the program closes
        if 'YES'.find(another.upper()) == -1:
            escape()

        same = input("Is it the same function? ")
        if same.upper() == "END":
            escape()

        while re.match(r'(YES|NO)', same.upper()) == None:
            if same.upper() == "END":
                escape()
            print('Please enter only "yes" or "no"')
            same = input("Is it the same function? ")

        if 'YES'.find(same.upper()) == -1:
            break # Breaks loop on line 79, loops to line 33

This is my first program that runs continuously until the user tells it to stop. I'm a beginner to Python and programming in general, so mostly I would love any advice on general style.

I'm sure there are more efficient ways to accomplish the goal, too. For example, I wanted the user to be able to exit the program at any time. However, since I'm using different variables for all of the inputs, the only solution that I could come up with was to put the escape() function under each input. Is there a simpler way to do this?

As for more advanced topics, I want to be able to check that the f(x) function is valid, but there are so many valid options. I just learned about regular expressions today, so I thought I'd start figuring out how to use them to check the function. I would also love to allow the user to enter more standard math inputs ("^" for exponents, 2x for multiplying "2" and "x", etc). Would regular expressions be the way to accomplish that, or is there a better option?

Thank you so much in advance for your critique! I'm proud of what I've made so far but I know it can be much better!

\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code review, please edit your title so that it only states the task accomplished by your code, anything else belongs to the body of the question \$\endgroup\$ – Parekh Nov 26 '20 at 7:00
  • 1
    \$\begingroup\$ A few suggestions for practice since you seem to be on the right track: Could you pull all the input checking into a separate function called once per loop? Could you map each input to a function using dict? How about add the choice to select between either a riemann sum or monte carlo and then implement a Monte Carlo simulation? None of those should be particularly hard and all would give you good practice \$\endgroup\$ – Coupcoup Nov 26 '20 at 19:36
7
\$\begingroup\$

There are a couple of bad patterns in here, for what is an otherwise not bad looking code for a beginner.

The most important of them is to not do the following (this is an example):

def do_something():
    something = another_variable_defined_below * 2

# bunch of code

something=3

So, looking at :

def right_riemann_sum():
    total_area = 0
    for k in range(rectangles):
        x = left_bound + (k + 1) * delta_x
        total_area += delta_x * eval(function.replace("x", str(x)))
    print('The area under the curve is %s' % total_area)

I'm wondering where do rectangles, left_bound, delta_x, function are coming from? In general, unless you're using some constants that are defined at the beginning of your code or you're inside a class, if a function needs to use some variables, they should be parameters to the method :

def right_riemann_sum(rectangles, left_bound, delta_x, function):

Because otherwise it gets confusing, what are the value of these parameters, how do I know if they're modified elsewhere or if they can be None.

Next, you'll want to have a better name for the variable rectangles (the other ones are looking good from what I saw). For example, rectangles is misleading. rectangles would be a good name for a list of rectangles. But in the case of the Riemann's sum, that variable contains a number of rectangles, so number_of_rectangles or num_rectangles would be more appropriate.

Try not to repeat yourself when possible. There is obvious duplicated code when asking for left and right bound, so why not create a function?

def ask_for_bound(bound_name):
    question = "What is the {0} bound? ".format(bound_name)
    bound = input(question)

    if bound.upper() == "END":
        escape()

    #Regex number check
    while re.match(r'(\A\-?\d*\.?\d+$)', bound) == None:
        if bound.upper() == "END":
            escape()

        print("Please enter only numbers.")
        bound = input(question)

    return float(left_bound)

Your usage of regex aren't bad, but the thing about regex is that they're hard to read : (\A\-?\d*\.?\d+$) checks for a floating point number. I would either :

  • compile the regex into a variable is_float_number = re.compile(r"(\A\-?\d*\.?\d+$)")
  • ditch the regex.

The regex works, that's good. But instead of working with the whole match/capture routine, you could consider that the user won't mess up and do this :

bound = 0
while True:
    try:
        bound = float(input(question))
        break
    except ValueError:
        pass

This way, you don't check if the string matches a floating point number, you just try to parse it and deal with the potential problem that the input isn't good.

There's another thing you should consider. Say you're filling a form on Google Forms and midway through you want to quit. Are you going to expect that there's a "quit" button near every entry of the form? You'll probably just close your browser tab and go do something else.

I'd think of it the same way with your program. Because right now you have soooo many if some_input == "END" : escape(). It clutters the code a lot. If you really want to leave the option for the user to quit at any moment, consider wrapping the input function this way :

def input_with_escape(question):
    response = input(question)

    if response.upper() == "END": 
        escape()

    return response

This way you won't have to check for END every time (Also, if you never specify to your user they can quit using END, they'll never use it).

Finally, replace while 1 == 1 by while True

Basically, try to keep your code that interacts with your user away from the code that has logic into it. Having print at the end of functions instead of return is usually not a good sign.

You've done a good job and you use most tools properly, there're just a couple of points you need to look after regarding clean code :)

\$\endgroup\$

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.