-
Notifications
You must be signed in to change notification settings - Fork 20k
Add itinerary tickets code #4340
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
Add itinerary tickets code #4340
Conversation
Imbiber
commented
Aug 30, 2023
- I have read CONTRIBUTING.md.
- This pull request is all my own work -- I have not plagiarized it.
- All filenames are in PascalCase.
- All functions and variable names follow Java naming conventions.
- All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.
src/main/java/com/thealgorithms/datastructures/hashmap/hashing/ItineraryTickets.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add some JUnit tests? (see https://github.com/TheAlgorithms/Java/tree/master/src/test/java/com/thealgorithms)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good. Please fix PR check https://github.com/TheAlgorithms/Java/actions/runs/6054455394/job/16431883329?pr=4340
There is some excessive whitespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general code looks good - no need for some big changes. Here are my small suggestions (some of them will fix the CI errors). You probably know its - It is enough to click Commit suggestion
button.
src/main/java/com/thealgorithms/datastructures/hashmap/hashing/ItineraryTickets.java
Outdated
Show resolved
Hide resolved
src/main/java/com/thealgorithms/datastructures/hashmap/hashing/ItineraryTickets.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/hashmap/hashing/ItineraryTicketsTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/thealgorithms/datastructures/hashmap/hashing/ItineraryTicketsTest.java
Outdated
Show resolved
Hide resolved
…/ItineraryTickets.java Co-authored-by: Piotr Idzik <[email protected]>
…/ItineraryTicketsTest.java Co-authored-by: Piotr Idzik <[email protected]>
…/ItineraryTicketsTest.java Co-authored-by: Piotr Idzik <[email protected]>
…/ItineraryTickets.java Co-authored-by: Piotr Idzik <[email protected]>
@vil02 can you help me out. I tried, but failed to correct the error last time , you providing me changes has given me hope. |
@Imbiber regarding clang-format, please have a look at: #4438 (comment) It seems that you need to change the order of the imports (cf. log) - this can be done by hand. |
src/test/java/com/thealgorithms/datastructures/hashmap/hashing/ItineraryTicketsTest.java
Outdated
Show resolved
Hide resolved
for (String key : tickets.keySet()) { | ||
revTickets.put(tickets.get(key), key); | ||
} | ||
for (String key : tickets.keySet()) { | ||
if (!revTickets.containsKey(key)) { | ||
return key; // Starting point | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about such renaming?
for (String key : tickets.keySet()) { | |
revTickets.put(tickets.get(key), key); | |
} | |
for (String key : tickets.keySet()) { | |
if (!revTickets.containsKey(key)) { | |
return key; // Starting point | |
} | |
for (final var startStation : tickets.keySet()) { | |
revTickets.put(tickets.get(startStation), startStation); | |
} | |
for (final var startStation : tickets.keySet()) { | |
if (!revTickets.containsKey(startStation)) { | |
return startStation; // Starting point | |
} |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution! |
Please reopen this pull request once you have made the required changes. If you need help, feel free to ask in our Discord server or ping one of the maintainers here. Thank you for your contribution! |