0
\$\begingroup\$

I have a code that calculates Euclidean distance on a grid with border crossing or without that.

def distance(x1: int, y1: int, x2: int, y2: int, grid_size: int, border_cross: bool):
    if border_cross:
        min_x = min(abs(x1 - x2), grid_size - abs(x1) - abs(x2))
        min_y = min(abs(y1 - y2), grid_size - abs(y1) - abs(y2))
        return (min_x**2 + min_y**2) ** 0.5
    
    dist_x = (x1 - x2) ** 2
    dist_y = (y1 - y2) ** 2
    return (dist_x + dist_y) ** 0.5

Can this code be simplified or improved in terms of readability/performance?

\$\endgroup\$
1
  • \$\begingroup\$ What do you mean by "border crossing"? And is there a good reason not to have two separate functions? \$\endgroup\$ Nov 10 at 10:43

1 Answer 1

2
\$\begingroup\$
min_x = min(abs(x1 - x2), 
dist_x = (x1 - x2) ** 2

We can see the border cross algorithm and the standard algorithm are similar so we can change the if border_cross code to mutate the values.

def distance(x1: int, y1: int, x2: int, y2: int, grid_size: int, border_cross: bool):
    x = abs(x1 - x2)
    y = abs(y1 - y2)
    if border_cross:
        x = min(x, grid_size - abs(x1) - abs(x2))
        y = min(y, grid_size - abs(y1) - abs(y2))
    return (x ** 2 + y ** 2) ** 0.5

The code is also WET (you write the same code for x and y) so we can write a helper function to get the distance and just call the function twice.

def _distance(i: int, j: int, grid_size: int, border_cross: bool) -> int:
    return (
        abs(i - j)
        if not border_cross else
        min(abs(i - j), grid_size - abs(i) - abs(j))
    )

def distance(x1: int, y1: int, x2: int, y2: int, grid_size: int, border_cross: bool) -> float:
    x = _distance(x1, x2, grid_size, border_cross)
    y = _distance(y1, y2, grid_size, border_cross)
    return (x ** 2 + y ** 2) ** 0.5

Personally I'd remove one of border_cross or grid_size. By using using None to mean False and providing an integer to mean True.

def _distance(i: int, j: int, border_cross: int | None) -> int:
    return (
        abs(i - j)
        if border_cross is None else
        min(abs(i - j), border_cross - abs(i) - abs(j))
    )

def distance(x1: int, y1: int, x2: int, y2: int, border_cross: int | None) -> float:
    x = _distance(x1, x2, border_cross)
    y = _distance(y1, y2, border_cross)
    return (x ** 2 + y ** 2) ** 0.5
\$\endgroup\$
1
  • \$\begingroup\$ I think it's worth noting that (x**2 + y**2) ** 0.5 should be spelled math.hypot(x, y), the latter has better numerical properties and is written in C. \$\endgroup\$ Nov 12 at 0:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.