swcarpentry / python-novice-inflammation

Programming with Python
http://swcarpentry.github.io/python-novice-inflammation/
Other
300 stars 775 forks source link

Improvement: Defensive Programming - Assertions #473

Open miili opened 6 years ago

miili commented 6 years ago

function normalize_rectangle is not easy to comprehend for a beginner. Why should a rectangle be normalized?

http://swcarpentry.github.io/python-novice-inflammation/08-defensive/

I propose a more simple function scale_rectangle:

def scale_rectangle(rect, factor):
    ''' Scale a rectangle from its lower left corner '''
    assert len(rect) == 4, "Rectangle must have four coordinates x1, y1, x2, y2"
    x1, y1, x2, y2 = rect

    assert x1 <= x2, "Invalid X coordinates"
    assert y1 <= y2, "Invalid Y coordinates"

    return (x1, y1, x2*factor, y2*factor)
annefou commented 6 years ago

I agree with you. I am not sure it is necessary to have such complex function. However, the name of the function scale_rectangle looses its meaning with your simplified version. I guess the idea was also to have a meaningful function (in terms of content). I labeled this issue as both enhancement and discussion to get more inputs from others.

maxim-belkin commented 6 years ago

Interesting suggestion! fixing the function is straightforward (given its description):

return (x1, y1, x1 + (x2-x1)*factor, y1 + (y2-y1)*factor)

Of course, that's assuming factor is positive... However, do we need to check that x1<x2 and y1<y2? What if we use min(x1, x2) or min( (x1, y1), (x2, y2) )? Would that increase cognitive load?

skuschel commented 5 years ago

related to #622

skuschel commented 5 years ago

I believe that neither the summing numbers nor the rectangle example show good practice for a few reasons:

A good example would be:

I actually find it quite hard to come up with a good example myself, because most errors are raised in the function anyways without manual checking before. From my experience the typical case is checking the number of arguments.