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

Add Sleep Sort in Scala #4495

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

Conversation

Dani-DEV28
Copy link

@Dani-DEV28 Dani-DEV28 commented Feb 17, 2025

Congrats on taking the first step to contributing to the Sample Programs repository maintained by The Renegade Coder!
For simplicity, please make sure that your pull request includes one and only one contribution.

Please fill one of the sections below as applicable.
Please also add any other relevant information to the Notes section at the bottom.
You may delete or just ignore any other sections.
For more information please refer to our contributing documentation

I Am Adding a New Code Snippet in an Existing Language

  • I fixed Add Sleep Sort in Scala #4465
  • I did not include any extra folders/libraries
  • I named the pull request using Add {PROJECT} in {LANGUAGE} format

Other Notes

Modify the .gitignore to exclude .metals and .scala-build

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Dani-DEV28! Thanks for contributing to this project! We are a rather small team, so it may take some time to process this request. In the meantime, there are several ways you can make yourself a part of The Renegade Coder community. For instance, you can:

Thanks for your help!

Copy link
Collaborator

@rzuckerm rzuckerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at the project requirements. The error messages are incorrect. In order to avoid a lot of duplication, I'd recommend making invalidChecker just return true or false. If invalid, display the common error message.

@rzuckerm rzuckerm added enhancement Any code that improves the repo sleep sort See: https://sampleprograms.io/projects/sleep-sort/ labels Feb 17, 2025
@rzuckerm rzuckerm changed the title Add SleepSort in Scala Add Sleep Sort in Scala Feb 17, 2025
@Dani-DEV28 Dani-DEV28 requested a review from rzuckerm February 17, 2025 18:10
Copy link
Collaborator

@rzuckerm rzuckerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better, but still not right. Please see the project requirements, specifically, this section: Sleep Sort Invalid Tests

Copy link
Collaborator

@rzuckerm rzuckerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you have a type mismatch error

@Dani-DEV28 Dani-DEV28 requested a review from rzuckerm February 19, 2025 21:42
@Dani-DEV28
Copy link
Author

I don't if I understand the test cases too well, but I think I found a issues with the test generated:
@project_test(PROJECT_NAME) @pytest.mark.parametrize( ("in_params", "expected"), [ pytest.param('"4, 5, 3, 1, 2"', "1, 2, 3, 4, 5", id="sample input"), pytest.param( '"4, 5, 3, 1, 4, 2"', "1, 2, 3, 4, 4, 5", id="sample input: with duplicate" ), pytest.param( '"1, 2, 3, 4, 5"', "1, 2, 3, 4, 5", id="sample input: already sorted" ), pytest.param( '"9, 8, 7, 6, 5, 4, 3, 2, 1"', "1, 2, 3, 4, 5, 6, 7, 8, 9", id="sample input: reverse sorted", ), ], )

I believe my issues with the test stem from that the extra (') in the in_param, because I do know if manual type all the test case on my local build they sort, but the test report that my code doesn't pass.

I was wondering where can I go about modifying the test_sleep_sort.py file, because when I modify it directly, it regenerate to before modification after running the glotter test command

@rzuckerm
Copy link
Collaborator

The in_params are correct. They work for every other language that has implemented Sleep Sort. The purpose of the single quotes around the double quotes is just how glotter handle passing command line arguments into the docker container that it runs to run the sample for each test.

@rzuckerm
Copy link
Collaborator

If you are running glotter locally, when prompted for the parameter, do not use the single quote. For example, you would enter "9, 8, 7, 6, 5, 4, 3, 2, 1".

@rzuckerm
Copy link
Collaborator

According to this article, using Future with synchronized can lead to undefined behavior. I'm wondering if something like a common mutex could be used instead of synchronized.

@rzuckerm
Copy link
Collaborator

You might want to take a look at this. I know this is Java, not Scala, but from this it looks like that should be used instead of the Scala class.

@rzuckerm
Copy link
Collaborator

One final comment, when you are ready to do the final submission, please remove all the commented out code. It adds clutter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any code that improves the repo sleep sort See: https://sampleprograms.io/projects/sleep-sort/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Sleep Sort in Scala
2 participants