Skip to content

Conversation

@SadguruSai
Copy link

Hi @kdemmich,

I refactored the IsoBased.execute_routing() method to improve readability and maintainability.

What I did

  • Broke down execute_routing() into private helper methods:
    • _handle_destination(self, constraints_list)
    • _handle_waypoint(self, constraints_list)
    • _handle_normal_step(self)
    • _finalize_execution(self)
    • _select_best_route(self)
    • _increment_count() – helper to increment relevant counters
    • _decrement_count() – helper to decrement relevant counters
  • Added descriptive docstrings to all new helper methods
  • Main method now acts as an orchestrator
  • Verified that all existing tests pass (pytest tests/test_isobased.py -v)

Why

This structure makes the function easier to read, maintain, and extend.

I’m still a beginner, so any feedback or suggestions are welcome 🙂

Thanks!

Hi @kdemmich,

I refactored the `IsoBased.execute_routing()` method to improve readability and maintainability.

### What I did
- Broke down `execute_routing()` into smaller private helper methods:
  - `_handle_destination(self, constraints_list)`
  - `_handle_waypoint(self, constraints_list)`
  - `_handle_normal_step(self)`
  - `_finalize_execution(self)`
  - `_select_best_route(self)`
- Main method now acts as an orchestrator
- Verified that all existing tests pass (`pytest tests/test_isobased.py -v`)

### Why
This structure makes the function easier to read, maintain, and extend.

I’m still a beginner, so any feedback or suggestions are welcome 🙂

Thanks!
@kdemmich
Copy link
Collaborator

kdemmich commented Oct 8, 2025

Hi @SadguruSai, thank you for your PR. We appreciate the time and effort that was put into it but, unfortunately, we decided to decline your PR as the features that it implements are already addressed by PR 60.

In this context, I would like to take the chance to point you to our updated guidelines for software contributions which require to follow 52 North's CLA guidelines for all future PRs.

@kdemmich kdemmich closed this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants