At the bottom is a new version, addressing most of the issues you're wondering about. But let us give a review to explain the changes:
- Intermixing functions and code – In your
on_touch_up you are intermixing code with the new functions. That is not good. You can introduce functions within a functions, but if you do so, then do it at the top of the function, so that it is very clear where you actual function body starts, and where the other functions are defined.
Alternative to function within function – There does however exist alternatives to those recursive function definition, and I'll present two lambda's and partial functions:
functools.partial – The other inside function you had was the call to line_expand(), and this would be nicer, in my opinion, if you put in the class scope. This does however need for more information to be passed, i.e. the touch data structure, so the definition for it to be a valid function is:
def _line_expand(self, touch, dt):
However, this is also used as a callback, so it should only take that one parameter, i.e. dt. To accomodate both these demands, you can use functools.partial to prefill a parameter like this:
growclock = Clock.schedule_interval(partial(self._line_expand, touch), 0.01)
lambda – In some cases, like a callback which will actually only do that one thing, you could use lambda's which is kind of a fake function for one expression.
Clock.schedule_once(lambda dt: Clock.unschedule(growclock), 1)
These two tricks allows you to remove both of the inner functions in your original on_touch_up(), and the full alternate version is presented below as alt_on_touch_up()
Combine the animation sequence into a method – Instead of having to always combine the two operations you could make a method which does the animation for you, with the given interval time, and the end time. This is presented below in the animate() function. Now the kind of 'messy' Clock.schedule stuff is hidden away in that method.
- Alternate ways to change a tuple – A tuple in Python is immutable, or unchangeable. You have to replace it, but how you replace it is another story. In the new
_line_expand() I'll show two alternate methods, one which lifts out the values from the tuple to dedicated values, and one which makes an alias for the touch.ud['ellipse'] part, which makes the changes a little easier on the eye.
- Comments are good! – Try working on giving some comments or docstrings on your code, as it will help you and others understand what happens.
So without further ado, here is the revamped version of class Test:
from kivy.app import App
from kivy.uix.widget import Widget
from kivy.graphics import Color, Rectangle, Line, Ellipse
from kivy.clock import Clock
from functools import partial
class Test(Widget):
def _line_expand(self, touch, dt):
"""Expands the size of the ellipse, and width of line"""
# Extract values in front of setting new
width, height = touch.ud['ellipse'].size
touch.ud['ellipse'].size = (width +1, height +1)
# Create an alias for later use
ellipse = touch.ud['ellipse']
ellipse.pos = (ellipse.pos[0] - 0.5,
ellipse.pos[1] - 0.5)
# ellipse.size = (ellipse.size[0] + 1, ellipse.size[1] +1)
touch.ud['line'].width += 0.5
def on_touch_down(self, touch):
"""Clear canvas, and create an ellipse and line starting at touch down point."""
with self.canvas:
Color(0, 0, 0)
Rectangle(pos=(0, 0), size=(self.width, self.height))
Color(1, 1, 1)
touch.ud['ellipse'] = Ellipse(pos=(touch.x - 10, touch.y - 10), size=[20, 20])
touch.ud['line'] = Line(points=[touch.x, touch.y], width=10)
def on_touch_move(self, touch):
"""Add points to the line when dragging mouse"""
touch.ud['line'].points += [touch.x, touch.y]
def on_touch_up(self, touch):
"""When releasing touch, call animation to expand line and ellipse."""
self.animate(partial(self._line_expand, touch), 0.01, 1)
def alt_on_touch_up(self, touch):
"""First rewrite of on_touch_up."""
growclock = Clock.schedule_interval(partial(self._line_expand, touch), 0.01)
Clock.schedule_once(lambda dt: Clock.unschedule(growclock), 1)
def animate(self, animate_func, interval_time, end_time):
"""Schedule animate_func every interval_time for end_time seconds."""
animation = Clock.schedule_interval(animate_func, interval_time)
Clock.schedule_once(lambda dt: Clock.unschedule(animation), end_time)
The TestApp and main call code is untouched, so keep them as they were. Hope this gives some hint on how to avoid or make it cleaner when using inner functions.
