1

What would be a pretty way to indent/format the line in the function below? Or should I not be trying to write this as a one liner at all?

def rects_bound(rects):
    """ Returns a rectangle that bounds all given rectangles
    """
    return reduce(lambda (ax,ay,aw,ah), (bx,by,bw,bh): (min(ax,bx),
                                                        min(ay,by),
                                                        max(ax+aw, bx+bw), 
                                                        max(ay+ah, by+bh)), rects)

or maybe

def rects_bound(rects):
    """ Returns a rectangle that bounds all given rectangles
    """
    return reduce(lambda (ax,ay,aw,ah), 
                         (bx,by,bw,bh): (min(ax,bx), min(ay,by),
                                         max(ax+aw, bx+bw), max(ay+ah, by+bh)), 
                  rects)

I usually just kind of "get creative" in these situations, and I know there is probably no "right" way, I'm just interested in your opinions and habits.

3
  • -1 Your lambda function is wrong (see my answer) ... this trumps any concerns about line length etc. Jan 6, 2012 at 20:37
  • Excuse me? My question was not whether the function does what it says it does, though you are right that it is 'wrong'. My question was about how to format this nicely.
    – noio
    Jan 6, 2012 at 21:51
  • You asked for opinions. You got one. Fix your data structure if you can. Write a correct reducer function. Then worry about line length. Jan 6, 2012 at 22:51

5 Answers 5

4

First of all, avoid long lines if possible. This particular example could be written much more readably as

def rects_bound(rects):
    x0 = min(x for x, y, w, h in rects)
    y0 = min(y for x, y, w, h in rects)
    x1 = max(x + w for x, y, w, h in rects)
    y1 = max(y + h for x, y, w, h in rects)
    return x0, y0, x1, y1

If you prefer to avoid the variables, you could also use

def rects_bound(rects):
    return (min(x for x, y, w, h in rects),
            min(y for x, y, w, h in rects),
            max(x + w for x, y, w, h in rects),
            max(y + h for x, y, w, h in rects))

which I still find much more readable than your original code.

(Note that I assume rects allows multiple iterations.)

3
  • @Noio: What's your problem with four iterations? (For what it's worth, my version is twice as fast as yours on my machine.) Jan 6, 2012 at 23:32
  • @SvenMarnach One problem is that in your version one can no longer pass a generator as the rects argument.
    – Bolo
    Aug 1, 2012 at 16:23
  • @Bolo: Yes, this is what the comment at the end of this answer means. Aug 1, 2012 at 16:29
2

If you're worried about long lines, don't use a lambda. Use a regular named function instead.

def rects_bound(rects):
    """ Returns a rectangle that bounds all given rectangles
    """
    def bounding_rect_reducer((ax, ay, aw, ah), (bx, by, bw, bh)):
        return (min(ax,bx),
                min(ay,by),
                max(ax+aw, bx+bw), 
                max(ay+ah, by+bh))

    return reduce(bounding_rect_reducer, rects)
2

I think this depends on the programmer and on the situation but I usually don't like to allocate a variable just to have shorter lines.

Looking at your 2 examples, I would go for the second one, or this one:

def rects_bound(rects):
    """ Returns a rectangle that bounds all given rectangles
    """
    return reduce(
      lambda (ax,ay,aw,ah), (bx,by,bw,bh):
        (min(ax,bx), min(ay,by), max(ax+aw, bx+bw), max(ay+ah, by+bh)
      ), 
      rects
    )
2
  • 2
    Is there any reason to not allocate a variable? I find that for long lines, assigning a good name to some result makes the code much more obvious. Plus function locals are pretty cheap. Jan 6, 2012 at 18:30
  • No reason at all. It's a personal choice, just like splitting long lines. I usually prefer to have a good function name that should describe what it does (in the name or above comments).
    – dresende
    Jan 12, 2012 at 15:27
1

Your lambda function is wrong. To fix it would make it even longer and involve redundant computation. Use a def instead:

def rects_bound(rects):
    """ Returns a rectangle that bounds all given rectangles
    """
    def bound_2_rects((ax, ay, aw, ah), (bx, by, bw, bh)):
        x = min(ax, bx)
        y = min(ay, by)
        return x, y, max(ax+aw, bx+bw) - x, max(ay+ah, by+bh) - y

    return reduce(bound_2_rects, rects)
0

I would suggest the following:

def rects_bound(rects):
    """ Returns a rectangle that bounds all given rectangles
    """
    return reduce(lambda (X,Y,W,H), (x,y,w,h): (min(X,x), min(Y,y),
                                                max(X+W, x+w),
                                                max(Y+H, y+h)), rects)

Reducing each argument to a single character really saves on space which helps this look cleaner. Another option would be to define the lambda function on a separate line (possibly even with def) so that the arguments aren't that far over in the first place.

3
  • @JohnMachin - Good point, added an extra line to cut it down. Jan 6, 2012 at 19:04
  • It's still fugly: the indentation has nothing to do with the syntax structure; it appears to be solely based on the the lines not being too long. Jan 6, 2012 at 19:35
  • 2
    @JohnMachin - Indenting just past the opening parentheses in a tuple is pretty standard for long lines. Jan 6, 2012 at 19:45

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.