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

Remove duplicate definition of LocalSearch #638

Merged
merged 2 commits into from
Jan 4, 2022

Conversation

jonathf
Copy link
Collaborator

@jonathf jonathf commented Dec 31, 2021

Issue

#634:
Python requires a large list of includes in memory at all given times. Using the same name in the same namespace causes compile issues.

The proposed minimal solution is to just rename the conflicting variable, but removing the name all together is also possible here as it is only used for convinience.

Tasks

  • Update CHANGELOG.md (remove if irrelevant)
  • review

@jcoupey
Copy link
Collaborator

jcoupey commented Dec 31, 2021

Thanks for the PR. If I get it right, the conflict is between the local LocalSearch type defined with "using" in vrptw.cpp and the one defined in src/algorithms/local_search/local_search.h? Then the same problem probably exists within src/problems/cvrp/cvrp.cpp?

What if we use a more specific local name, e.g. using VRPTWLocalSearch = ... and using CVRPLocalSearch = ... in vrptw.cpp and cvrp.cpp respectively? Or maybe even better we can namespace them as vrptw::LocalSearch and cvrp::LocalSearch?

@jcoupey jcoupey added this to the v1.12.0 milestone Dec 31, 2021
@jonathf
Copy link
Collaborator Author

jonathf commented Jan 1, 2022

Yes, that follows. pyvroom isn't feature complete yet, so it will likely not catch everything round one.

I like the namespace solution best. I can implement it if you like.

@jonathf
Copy link
Collaborator Author

jonathf commented Jan 1, 2022

It is update now, @jcoupey.

I am realizing through the trenches of undeciphrable C++ error messages, that the name conlfict might not be against src/algorithms/local_search/local_seach.h since it is already in another namesspace. The conflict is more likely between cvrp.cpp and vrptw.cpp which both uses the root vroom namespace. Either way, butting both in seperate namespaces solves the issue on my side.

@jcoupey jcoupey merged commit e2ef70e into VROOM-Project:master Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants