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, 2023 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, 2023 at 0:42

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.