8
\$\begingroup\$

I have made some code which will validate user input for getting 4 control points for a bezier curve. It works fine however I was just wondering If I could make this code any more compact or efficient?

p = [] #empty points array

#gets 4 control points from user input
def get_points():
    #loops through 4 times to get 4 control points
    points = ["Start Point(P0)", "Control Point 1(P1)", "Control Point 2(P2)", "End Point(P3)"]
    for i in range(4):
        while True:
            #user input
            p_input = input("Enter X,Y Coordinates for "+ points[i]+":")
            #splits the string into x and y coordinates
            p_components = p_input.split(',')
            #checks to see if user hasnt entered two coordinates
            if len(p_components) != 2:
                print("Missing coordinate please try again.")
                p_input = input("Enter X,Y Coordinates for "+ points[i]+":")
                p_components = p_input.split(',')
            #checks to see if the values can not be converted into floats
            try:
                x = float(p_components[0])
                y = float(p_components[1])
            except ValueError:
                print("Invalid coordinates", p_components, "please try again.")
            else:
                #makes sure user is not entering coordinates that will not appear on the screen.
                if(x > 1020 or x < 10) or (y > 760 or y < 10): 
                    print("Invalid coordinates, please try again.")
                else:
                    #appends the x and y coordinates as a 2 dimensional array
                    p.append([float(p_components[0]), float(p_components[1])])
                    break
\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

There are things that can be improved:

  • You could return the points instead of asigning the global variabel p
  • You could split the function in two functions:
    1. get_four_bezier_points
    2. get_point
  • it probably would be better to return float values instead of string values.
  • You could inform the user that the input x and y values should be between certain values and must be separated with a comma.
  • the error checking logic's could be restructured such that the second input statement can be removed.

Putting these together as an example:

def get_point(description):
    while True:

        p_input = input("Enter X,Y Coordinates for " + description + ": ")
        p_components = p_input.split(',')

        if len(p_components) != 2:
            print("Not two comma seperated values, please try again")
            continue # (jumps to the beginning of the while loop)

        try:
            x = float(p_components[0])
            y = float(p_components[1])
        except ValueError:
            print("No float values, please try again.")
            continue

        if(x > 1020 or x < 10) or (y > 760 or y < 10): 
            print("Values out of bounds.")
            continue

        # oke values are good
        return [x,y]  # jumps out of the function and while loop 

def get_four_bezier_points():
    p = []
    print("Enter four bezier points with x in [10,1020] and y in [10,760]")
    points = ["Start Point(P0)", "Control Point 1(P1)", "Control Point 2(P2)", "End Point(P3)"]
    for description in points:
        p.append(get_point(description))
    return p

p = get_four_bezier_points()
\$\endgroup\$
5
  • \$\begingroup\$ If I were to return the points, then everytime I wanted to use the points I would have to call get_points and it would ask for user input every time. How would I go about splitting the function up? \$\endgroup\$
    – user160134
    May 12, 2018 at 14:00
  • \$\begingroup\$ @Zestyy99, you could call the function and assign the result to p as in p = get_points(). \$\endgroup\$
    – Jan Kuiken
    May 12, 2018 at 14:04
  • \$\begingroup\$ @Zestyy99, i edited my answer and added an example \$\endgroup\$
    – Jan Kuiken
    May 12, 2018 at 14:18
  • \$\begingroup\$ thank you, that looks a lot better than the original :) \$\endgroup\$
    – user160134
    May 12, 2018 at 14:24
  • 1
    \$\begingroup\$ Or even better imo: return [get_point(description) for description in points] \$\endgroup\$
    – Draconis
    May 12, 2018 at 18:42
4
\$\begingroup\$
  • for the first test len(p_components) != 2 you can enter anything the second time.
  • your error messages are vague, they don't help the user to input good values

you could do something like that:

p = [] #empty points array

#gets 4 control points from user input
def get_points():
    #loops through 4 times to get 4 control points
    points = ["Start Point(P0)", "Control Point 1(P1)", "Control Point 2(P2)", "End Point(P3)"]
    for i in range(4):
        is_good_entry = False
        while not is_good_entry:
            p_input = input("Enter X,Y Coordinates for "+ points[i]+":")
            p_components = p_input.split(',')

            if len(p_components) != 2:
                print("Wrong number of coordinates, please try again.")
                continue

            try:
                x = float(p_components[0])
                y = float(p_components[1])
            except ValueError:
                print("Only numbers are allowed")
                continue

            if(x > 1020 or x < 10) or (y > 760 or y < 10):
                print("Wrong range. x must be in range [10, 1020] and y [10, 760]")
                continue

            is_good_entry = True
            p.append([x, y])

get_points()
\$\endgroup\$

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.