11
\$\begingroup\$

Physicist Richard Feynman though of a very interesting riddle:

Imagine you are a lifeguard at the side of the beach and there is a child drowning in the water. (You must cross both sand and water to get to him)

Which route would you take to go to the child and save him?

Not a straight line1 because in the sand you move faster than in the water so the advantage of staying a larger percentage of the trip on the sand outweighs the disadvantage of having a longer trip (straight line is shortest) up to a certain point.

How to get an intuition about the shape of such route?

I wrote a simple visualization with modifiable parameters to get an intuitive feeling for different start positions and sand/sea speed ratios, here is an example (yellow is sand, blue is sea):

enter image description here

I am interested in all kinds of suggestions, here is the code:

from tkinter import *

LIFEGUARD_Y = 100
CHILD_Y = 350
SAND_SPEED = 3 # SEA_SPEED = 1
WIDTH = 500
HEIGHT = 500

def distance(a, b):
    return ((a[0] - b[0])**2 + (a[1]-b[1])**2)**0.5

def total_time(lifeguard_y, child_y, midpoint_y,sand_speed=SAND_SPEED):
    sand_len = distance( (lifeguard_y, 0), (midpoint_y, WIDTH//2) )
    sea_len = distance( (midpoint_y, WIDTH//2), (child_y, WIDTH) )
    sand_time = sand_len / sand_speed
    sea_time  = sea_len / 1
    return sand_time + sea_time

def find_optimal_midpoint_y(lifeguard_y, child_y,sand_speed=SAND_SPEED):
    return min(range(0, 500), key=lambda y: total_time(lifeguard_y, child_y, y, sand_speed))

def show_situation():
    master = Tk()

    master.title("Feymann lifeguard route problem")
    w = Canvas(master, width=WIDTH, height=HEIGHT)
    w.pack()


    l = Label(master, text="Lifeguard Y")
    l.pack()

    lifeguard_y_w = Scale(master, from_=0, to=HEIGHT, orient=HORIZONTAL)
    lifeguard_y_w.pack()

    l = Label(master, text="Child Y")
    l.pack()

    child_y_w = Scale(master, from_=0, to=HEIGHT, orient=HORIZONTAL)
    child_y_w.pack()

    l = Label(master, text="Sand Speed / Sea Speed ratio")
    l.pack()

    sand_speed_w = Scale(master, from_=1, to=3, resolution=0.1, orient=HORIZONTAL)
    sand_speed_w.pack()


    w.create_rectangle(0, 0, WIDTH//2, HEIGHT, fill="yellow")
    w.create_rectangle(WIDTH//2, HEIGHT, WIDTH, 0, fill="blue")

    def draw_optimal_route():
        w.create_rectangle(0, 0, WIDTH//2, HEIGHT, fill="yellow")
        w.create_rectangle(WIDTH//2, HEIGHT, WIDTH, 0, fill="blue")

        midpoint_y = find_optimal_midpoint_y(lifeguard_y_w.get(), child_y_w.get(), sand_speed=sand_speed_w.get())

        w.create_line(0, lifeguard_y_w.get(), WIDTH//2, midpoint_y)
        w.create_line(WIDTH//2, midpoint_y, WIDTH, child_y_w.get())

    b = Button(master, text="GO", command=draw_optimal_route)
    b.pack()

    mainloop()

if __name__ == "__main__":
    show_situation()

1 A straight line only in the trivial and non-realistical case where sand-speed = sea-speed such as for a bird.

Bonus Physics Note: Light behaves the same way as this perfect lifeguard because it always chooses the fastest path.

\$\endgroup\$
  • 5
    \$\begingroup\$ More specifically, this is the principle behind Snell’s Law. \$\endgroup\$ – 200_success Sep 22 '17 at 13:23
  • \$\begingroup\$ That's a nice info @200_success! \$\endgroup\$ – Gabriel Sep 22 '17 at 19:11
  • 2
    \$\begingroup\$ Would it be worthwhile to mention that this very problem is typically one of the first you solve while learning calculus, so that you can get the exact answer immediately instead of testing 500 possible values? \$\endgroup\$ – Snowbody Sep 25 '17 at 15:11
6
\$\begingroup\$

Don't use wildcard imports

Change from tkinter import * to import tkinter as tk, and then prefix all tk classes and variables with tk. (eg: tk.Frame(...)).

One reason is that PEP8 discourages wildcard imports. Also, by using the prefix your code becomes more self-documenting.

One problem I see people make is that they import * from both tkinter and ttk. Since both of them have many classes with the same name (Button, Entry, etc), it becomes impossible to know which is being used without remembering which the order of the imports. If you change the order of the imports you could break many lines of your code.

You don't use ttk here, but if you get in the habit of using wildcard imports, eventually it will cause you problems.

Name your main function main

This is largely personal preference, but I think it helps to name your main function main, and place it at the top of the file. When I was scanning your code, it wasn't until I got down to literally the last line in the file that I knew which function was called first.

Use an object-oriented architecture

Python objects are easy to use, and make writing GUI code much easier. I recommend writing all tkinter programs with classes and objects. There's a stackoverflow question that covers this: Best way to structure a tkinter application

Separate widget creation from widget layout

I personally find it easier to write, maintain, and understand code when the widget creation is separate from widget layout. That is, instead of doing "create widget; layout widget; create widget; layout widget", do "create widget; create widget; layout widget; layout widget". I usually do the grouping by parent (eg: create all children of a parent widget, then layout those children, then go to the next, and so on)

When all of your widgets are in the root it's not quite as important as when you have multiple areas in your GUI that are independent, but even with everything sharing the same parent, grouping them together makes that more apparent.

For example:

l = Label(master, text="Lifeguard Y")
lifeguard_y_w = Scale(master, from_=0, to=HEIGHT, orient=HORIZONTAL)
l = Label(master, text="Child Y")
child_y_w = Scale(master, from_=0, to=HEIGHT, orient=HORIZONTAL)
l = Label(master, text="Sand Speed / Sea Speed ratio")
sand_speed_w = Scale(master, from_=1, to=3, resolution=0.1, orient=HORIZONTAL)

l.pack()
lifeguard_y_w.pack()
l.pack()
child_y_w.pack()
l.pack()
sand_speed_w.pack()

Use explicit options to pack

You are relying on default values for all of your pack statements. While those defaults likely won't ever change, it makes your code a bit easier to read and your intent more obvious if you're explicit about where you expect the widgets to go.

l.pack(side="top")
lifeguard_y_w.pack(side="top")
l.pack(side="top")
child_y_w.pack(side="top")
l.pack(side="top")
sand_speed_w.pack(side="top")

Consider the effect of resizing the window on your design

Your program doesn't handle resizing very well. It's not terrible, but it could be better. For example, as a user I would expect the yellow/blue area to expand to fill the top part of the window when I make the window larger, but it doesn't. I also don't expect to have the buttons chopped off if I make the window smaller; instead, I expect the yellow/blue area to shrink. It's also fairly common for an "OK" button to stick to the bottom, which it doesn't.

You can use options to the geometry managers (pack, place, and grid) to control this. There's no single solution to making it work, it all depends on what you want to have happen when the window is resized. However, the first step usually involves designating a "hero" widget that will grow and shrink with the window. Most GUIs have such a thing, typically it's a text or canvas widget.

\$\endgroup\$
  • \$\begingroup\$ tkinter is an exception for wildcard imports because it is very prevalent, about the object oriented structure I have doubts because such coding patterns give better results when the application is complex and this is pretty simple. The comment about resizing is interesting and surely makes for a better user experience but I fear that the complications would be substantial (maybe, I do not know) \$\endgroup\$ – Caridorc Sep 22 '17 at 17:49
  • 2
    \$\begingroup\$ @caridork I disagree with your comments about importing. Tkinter is the poster child for why wildcard imports are bad since it and ttk have many identically named classes. There is simply no good reason to use wildcard imports with Tkinter because it is a source of potential bugs and reduces clarity. Thanks for your comments though. It is good food for thought. \$\endgroup\$ – Bryan Oakley Sep 22 '17 at 17:59
  • \$\begingroup\$ I think you are correct about imports, I used wildcard imports because my source website (effbot) used them so given how useful that website examples are I thought it was good standard practice to use such imports, but I can understand the possible confusion in larger programs. Even here I guess the tradeoff is how much do you really expect your project to grow, if it is always going to be less than 100 lines toy project wildcard is probably fine, but I guess somewhere over 500 you could start having bad trouble. Interesting comments all around. What do you think of packing widgets in a loop? \$\endgroup\$ – Caridorc Sep 22 '17 at 18:10
  • 3
    \$\begingroup\$ The OP is right about the note inherent to the imports. I saw some people on StackOverflow writing from ttk import * and then from tkinter import * so they end up by not getting benefit of the themed widgets because the second import overrides the 11 widgets ttk that shares them with tkiner. @Caridorc \$\endgroup\$ – Billal Begueradj Sep 22 '17 at 19:17
  • 1
    \$\begingroup\$ I understand you because there are many old blogs which are poorly written and teach bad habits for programmers @Caridorc \$\endgroup\$ – Billal Begueradj Sep 22 '17 at 19:21
5
\$\begingroup\$

Performance issue

The callback you are using for the b button forces your application to redraw the sea and sand rectangles as many times as the user clicks on the button. You are not supposed to redraw any widget or graphical elements within the canvas when you call draw_optimal_route() except the line which is the only useful information the user expects from your application. This is a real performance issue you must think about.

UX issue

On my laptop, I hardly am able to run your program because the vertical length of the window does not allow me to see the GO button. The best user experience I can get from the GUI is this one:

enter image description here

Of course, this is not an issue if the resolution of your laptop is better than mine.

Design issue

OOP and GUIs are good friends. No wonder that, historically speaking, graphical user interfaces coincided with the birth of object-oriented concepts (source):

Sketchpad can be considered as the first Graphical User Interface (GUI) ... while the Object-Oriented term itself is formed 4 years later (in 1966). My guess is that Alan Kay is looking back at those days and he made Sketchpad as aspiration of what will become of OOP)

This means, at your place, I would start by object-orient that program.

I know there are many Gods out there who say OOP is not good. But I can assure you that OOP is at least your best friend when you deal with graphical user interfaces. If you do not believe me, look how many engineering principles we respect and problem we solve by re-designing your program using the OOP approach:

Use as less parameters as possible

My solution below renders most of your functions niladic (having no argument) and, at worst, few of them are monadic (having one argument). This is an extremely good design principle for 2 main reasons: 1. Parameters belong to a level of abstraction which is different from that of functions. 2. Less parameters makes your tests (such as unit tests) much easier.

Getting rid of the performance issue

The use of OOP principles, allowed me to get rid of the performance issue mentioned in the beginning. This is achieved almost "by default".

Fixing the UX issue

I suggest you to redesign your GUI as shown in the below screenshot. This may be interesting for users with a machine like mine:

enter image description here

Random notes in brief

  1. Let us comply with PEP8:

Wildcard imports (from import *) should be avoided, as they make it unclear which names are present in the namespace, confusing both readers and many automated tools. There is one defensible use case for a wildcard import, which is to republish an internal interface as part of a public API (for example, overwriting a pure Python implementation of an interface with the definitions from an optional accelerator module and exactly which definitions will be overwritten isn't known in advance).

This means you could change from tkinter import * to import tkinter as tk.

  1. You can improve the names of different instances and variables you used. For a small program like that, you can allow yourself, maybe, to name a variable "w" but you can not provide this luxury for yourself in larger programs as you can not remember what they are referring to. Each variable or function name must be well chose to serve its purpose and reflect it as good as possible.
  2. I think it is a little bit confusing that "Sand Speed / Sea Speed ratio". Actually you commented that the "sea speed" is one, and you never referred to this notion any where in your program. So I suggest you to re-write the label text to refer only to the sand speed.
  3. I noticed in the same scope, you use the same name for several variables. This is the case, for instance, when you named all the labels by "l". You are lucky in this context you did not suffer from the consequences of such coding choice, but that is not an excuse you keep that attitude for your further applications.
  4. Misleading named parameter: In both definitions of find_optimal_midpoint_y() and total_time() functions, you set a defined parameter sand_speed=SAND_SPEED, but later when you inject sand_speed_w.get() as the actual argument when you call them through this line of code: midpoint_y = find_optimal_midpoint_y(... sand_speed=sand_speed_w.get()). Of course, you have the right to change the value of named parameter to whatever you want, but I just do not see the point of using it in this context because of two main reasons: first when I tried to redesign your program, I have been thinking to get rid of that named parameter and use directly SAND_SPEED within the body of the two functions instead. But then, I found you never used that constant at all, so simply get rid of it. The second thing about it that instead of coding midpoint_y = find_optimal_midpoint_y(... sand_speed=sand_speed_w.get()) you can simply write midpoint_y = find_optimal_midpoint_y(... sand_speed_w.get()) because you can use named parameters just as you use mandatory ones.
  5. Misuse of inner functions The pythonic purpose of inner functions is rather to devise decorators. Using inner functions notion in your context leads to the performance issue mentioned above.
  6. In addition to SEA_SPEED, you should get rid of LIFEGUARD_Y and CHILD_Y constants which seem to be useless for your program.

Solution

Given the points I mentioned above and some other minor things I could not write right now (I'm a little sick), I came up with this solution. You can refer to tkinter best practices article to understand the logic and, more importantly, please do not hesitate to comment anything you do not understand in my approach below. Note that I ended up with more code, but a cleaner and scalable one:

import tkinter as tk


WIDTH = 500
HEIGHT = 500

class FeymannLifeguard(tk.Frame):

   def __init__(self, master):
       self.master = master
       tk.Frame.__init__(self, self.master)
       self.configure_gui()
       self.create_widgets()

   def configure_gui(self):
       self.master.title('Feymann lifeguard route problem')
       self.master.geometry('500x600')
       self.master.resizable(False, False)

   def create_widgets(self):
       self.create_sea_sand_area()
       self.create_control_area()

   def create_sea_sand_area(self):
       self.sea_sand_area = tk.Canvas(self.master, width=WIDTH, height=HEIGHT)
       self.sea_sand_area.pack()
       self.sand_area = self.sea_sand_area.create_rectangle(0, 0, WIDTH//2, HEIGHT, fill='yellow')
       self.sea_area = self.sea_sand_area.create_rectangle(WIDTH//2, HEIGHT, WIDTH, 0, fill='blue')

   def create_control_area(self):
       self.control_area = tk.Canvas(self.master, width=WIDTH, height=100)
       self.control_area.pack()
       self.create_control_widgets()

   def create_control_widgets(self):
       self.lifeguard_label = tk.Label(self.control_area, text='Lifeguard')
       self.lifeguard_scale = tk.Scale(self.control_area, from_=0, to=HEIGHT, orient=tk.HORIZONTAL)
       self.child_label = tk.Label(self.control_area, text='Child')
       self.child_scale = tk.Scale(self.control_area, from_=0, to=HEIGHT, orient=tk.HORIZONTAL)
       self.sand_speed_label = tk.Label(self.control_area, text="Sand Speed")
       self.sand_speed_scale = tk.Scale(self.control_area, from_=1, to=3, resolution=0.1, orient=tk.HORIZONTAL)
       self.go = tk.Button(self.control_area, text='GO', command=self.draw_optimal_route)
       self.position_control_widgets()

   def position_control_widgets(self):
       self.lifeguard_label.grid(row=0, column=0, padx=0, sticky=tk.W)
       self.lifeguard_scale.grid(row=1, column=0, sticky=tk.W)
       self.child_label.grid(row=0, column=1, padx=20, sticky=tk.W)
       self.child_scale.grid(row=1, column=1, padx=20, sticky=tk.W)
       self.sand_speed_label.grid(row=0, column=2, padx=20, sticky=tk.W)
       self.sand_speed_scale.grid(row=1, column=2, padx=20, sticky=tk.W)
       self.go.grid(row=1, column=3, sticky=tk.S, padx=20)

   def distance(self, a, b):
       return ((a[0] - b[0])**2 + (a[1]-b[1])**2)**0.5


   def total_time(self, midpoint):
       sand_len = self.distance((self.lifeguard_scale.get(), 0), (midpoint, WIDTH//2))
       sea_len = self.distance((midpoint, WIDTH//2), (self.child_scale.get(), WIDTH))
       sand_time = sand_len/self.sand_speed_scale.get()
       sea_time  = sea_len / 1
       return sand_time + sea_time

   def find_optimal_midpoint(self):
       return min(range(0, 500), key=lambda y: self.total_time(y))

   def draw_optimal_route(self):      
       self.clear_sea_sand_area()
       self.midpoint_y = self.find_optimal_midpoint()
       self.sea_sand_area.create_line(0, self.lifeguard_scale.get(), WIDTH//2, self.midpoint_y, tag='sea_line')
       self.sea_sand_area.create_line(WIDTH//2, self.midpoint_y, WIDTH, self.child_scale.get(), tag='sand_line')

   def clear_sea_sand_area(self):
       self.sea_sand_area.delete('sea_line')
       self.sea_sand_area.delete('sand_line')

if __name__ == '__main__':
   root = tk.Tk()
   FeymannLifeguard(root)
   root.mainloop()
\$\endgroup\$
  • \$\begingroup\$ I have a doubt because I understand (maybe wrongly) that you say that I used the canvas widget as the parents for my label and slider widgets while in reality I used the master widget as their parent \$\endgroup\$ – Caridorc Sep 22 '17 at 19:27
  • \$\begingroup\$ Also I do not understand your point about functions with no arguments being easier to test than functions with more arguments. If a function takes a value from the class it is in it becomes harder to test compared to a function that takes all of its inputs as arguments \$\endgroup\$ – Caridorc Sep 22 '17 at 19:29
  • \$\begingroup\$ Sorry, you are right for your first comment, I removed the corresponding section. For the second comment: let us say we have function with 10 arguments: imagine how much different combinations we should test to cover all the test cases. This is exactly what you can read, for example, in a book called "Clean code", written by Robert C. Martin @Caridorc \$\endgroup\$ – Billal Begueradj Sep 22 '17 at 19:37
  • \$\begingroup\$ Many functions with multiple parameters have many different default values rarely to be changed. About the efficiency I think that it is not a problem because we are talking about drawing 2 rectangles each time the user presses a button, but the user should press the button very rarely in normal program operation even less than once per second. \$\endgroup\$ – Caridorc Sep 22 '17 at 20:04
0
\$\begingroup\$

I want to share with you the work of Amon (apparently, he is not interested to post his solution to here) who refactored my above solution by applying simple design concepts and ended by a better solution than mine.

Amon first published the solution here.

"""
Refactored from the FeymannLifeguard class
  by Billal BEGUERADJ
  at <https://codereview.stackexchange.com/a/176327/21609>
which was based on the question
   "Feynmann lifeguard riddle graphical representation"
   by Caridorc
   at <https://codereview.stackexchange.com/q/176298/21609>

Licensed under CC-BY-SA 3.0
  <https://creativecommons.org/licenses/by-sa/3.0/>
"""
import tkinter as tk

class SandAndSea(tk.Canvas):
    def __init__(self, *, master, width, height):
        super().__init__(master=master, width=width, height=height)
        self.pack()

        self.width = width
        self.sand_area = self.create_rectangle(
            0, 0, width//2, height, fill='yellow')
        self.sea_area = self.create_rectangle(
            width//2, height, width, 0, fill='blue')

    def draw_route(self, *, lifeguard, child, midpoint):
        self.clear_route()

        width = self.width
        self.create_line(0, lifeguard, width//2, midpoint, tag='sea_line')
        self.create_line(width//2, midpoint, width, child, tag='sand_line')

    def clear_route(self):
        self.delete('sea_line')
        self.delete('sand_line')


class ControlPanel(tk.Canvas):
    PADDING = 10

    def __init__(self, *, master, width, height):
        super().__init__(master=master, width=width, height=height)
        self.__col = 0

    def __grid_position(self, widget, *, row, sticky=tk.W, **kwargs):
        widget.grid(row=row, column=self.__col,
                    padx=self.PADDING, sticky=sticky, **kwargs)

    def add_scale(self, text, from_, to, step=1):
        label = tk.Label(self, text=text)
        scale = tk.Scale(self, from_=from_, to=to,
                         orient=tk.HORIZONTAL,
                         resolution=step)

        self.__grid_position(label, row=0)
        self.__grid_position(scale, row=1)
        self.__col += 1

        return scale

    def add_button(self, *args, **kwargs):
        button = tk.Button(self, *args, **kwargs)

        self.__grid_position(button, row=1, sticky=tk.S)
        self.__col += 1

        return button


def time_in_medium(dx, dy, speed):
    length = (dx**2 + dy**2)**0.5
    return length/speed


def optimal_midpoint(*, lifeguard, child, sand_speed, width):
    def total_time(midpoint):
        sand_time = time_in_medium(width//2, lifeguard - midpoint, sand_speed)
        sea_time = time_in_medium(width//2, midpoint - child, 1)
        return sand_time + sea_time

    start, end = (lifeguard, child) if lifeguard < child else (child, lifeguard)
    return min(range(start, end), key=total_time)


WIDTH = 500
HEIGHT = 500


class FeymannLifeguard(tk.Frame):
    def __init__(self, master):
        super().__init__(master=master)

        self.sea_sand_area = SandAndSea(
            master=master, width=WIDTH, height=HEIGHT)

        control_area = ControlPanel(master=master, width=WIDTH, height=100)
        control_area.pack()

        self.lifeguard  = control_area.add_scale('Lifeguard', 0, HEIGHT).get
        self.child      = control_area.add_scale('Child', 0, HEIGHT).get
        self.sand_speed = control_area.add_scale('Sand Speed', 1, 3, 0.1).get

        control_area.add_button(text='GO', command=self.draw_optimal_route)

    def draw_optimal_route(self):
        midpoint = optimal_midpoint(
            lifeguard=self.lifeguard(),
            child=self.child(),
            sand_speed=self.sand_speed(),
            width=WIDTH)

        self.sea_sand_area.draw_route(
            lifeguard=self.lifeguard(),
            child=self.child(),
            midpoint=midpoint)


def run_app():
    root = tk.Tk()
    root.title('Feymann lifeguard route problem')
    root.geometry('{}x{}'.format(WIDTH, HEIGHT + 100))
    root.resizable(False, False)

    FeymannLifeguard(root)

    root.mainloop();

if __name__ == '__main__':
    run_app();
\$\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.