Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add Traveling Salesman Problem algorithm and tests #717

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

harshithsaiv
Copy link

Implemented Traveling Salesman Problem using Dynamic Programming

@harshithsaiv
Copy link
Author

@TruongNhanNguyen can you please review the code

@TruongNhanNguyen
Copy link
Contributor

@harshithsaiv Sure, I can provide suggestions that you would like to apply into your code to improve it. I'm just a contributor to this repo and only have contribute rights (give suggestions to make the PR to be better, not the right to merge code into master branch).

@TruongNhanNguyen
Copy link
Contributor

Although your implementation is LGTM, some points in your code need to be changed or improved further.

  1. Variable Names: Variable names like n, dp, mask, i, and j are common in dynamic programming algorithms, but they could be more descriptive to enhance readability. For instance, n could be renamed to numCities, dp could be costTable, and so on.
  2. We can refactor the code to separate the logic into reusable functions:
    • initializeCostTable initializes the cost table with maximum costs.
    • updateCostTable updates the cost table with the minimum cost to reach each city.
    • findMinimumCost finds the minimum cost to complete the traveling salesman route.
    • The main logic for solving the traveling salesman problem is encapsulated within the TravelingSalesman function, which orchestrates the use of the helper functions.

@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.43%. Comparing base (833a3e5) to head (c2ff1c3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #717      +/-   ##
==========================================
+ Coverage   87.36%   87.43%   +0.07%     
==========================================
  Files         200      201       +1     
  Lines        5317     5350      +33     
==========================================
+ Hits         4645     4678      +33     
  Misses        533      533              
  Partials      139      139              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TruongNhanNguyen
Copy link
Contributor

LGTM.

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.

None yet

4 participants