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

Added Palindrome in Kotlin #2080

Merged
merged 16 commits into from
Oct 8, 2020
Merged

Added Palindrome in Kotlin #2080

merged 16 commits into from
Oct 8, 2020

Conversation

anohene1
Copy link
Contributor

@anohene1 anohene1 commented Oct 3, 2020

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.

Complete the Applicable Sections Below

Find which section best describes your pull request and make sure you fill it out. To start, let us know which issue you've fixed.

  • I fixed #your-issue-number-here

Code Snippets

  • I named the pull request using Added/Updated <Sample Program> in <Language> format
  • I created/updated the language README
    • I added the sample program name to the README
    • I added fun facts (i.e. debut developer, typing, etc.)
    • I added reference link(s) to the README
    • I added solution citations when necessary (see plagiarism)

Testing

  • I named the pull request using Added/Updated <Language>/<Project> Testing format
  • I followed the testinfo template, if applicable

Notes

Feel free to describe what you added or updated.

Algorithm to check if a number is a palindrome or not.
@jrg94 jrg94 changed the title Added palindrome checker in kotlin Added Palindrom in Kotlin Oct 7, 2020
@jrg94 jrg94 requested a review from a team October 7, 2020 03:27
@jrg94 jrg94 self-assigned this Oct 7, 2020
@jrg94 jrg94 added enhancement Any code that improves the repo needs docs New code that isn't documented here: https://sampleprograms.io/projects/ labels Oct 7, 2020
@jrg94 jrg94 added this to the 1,000 Code Snippets milestone Oct 7, 2020
@jrg94 jrg94 changed the title Added Palindrom in Kotlin Added Palindrome in Kotlin Oct 7, 2020
Copy link
Member

@jrg94 jrg94 left a comment

Choose a reason for hiding this comment

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

We don't currently support palindrome, but we're open to including this if we can get it documented. Let me know if that's something you want to do!

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 7, 2020

Sure. I'll do that. Let me know where to document it.

@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

Check out this repo: https://github.com/TheRenegadeCoder/sample-programs-website

You'll want to create something that looks like one of these: https://sample-programs.therenegadecoder.com/projects/

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 7, 2020

I have done that and made a pull request. You can check it at TheRenegadeCoder/sample-programs-website#349

@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

Cool! Now we have the specification, let's get some tests going. There's a bit of a dependency in our way at the moment. See #2186. That said, we can add the test file here: https://github.com/TheRenegadeCoder/sample-programs/tree/master/test

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 7, 2020

I'm sorry I don't get you quite well. Should the test be in python? As I can see all the test files are written in python.

@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

Yes! But the test files are more or less copy-and-paste. You should be able to copy binary-search.py and reuse most of it.

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 7, 2020

Okay I'll go ahead and try that. I'll let you know if I'm done.

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 7, 2020

This is getting very difficult for me as I have no knowledge in python.

@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

No worries! I can give you a rough idea of what you need to do. First, I'd copy the contents of the binary search test.

@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

Then, replace BinarySearch with PalindromicNumbers.

@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

Then, at the top of the file, you should see these:

invalid_permutations = (
    'description,in_params,expected', [
        (
            'no input',
            None,
            'Usage: please provide a list of sorted integers ("1, 4, 5, 11, 12") and the integer to find ("11")'
        ), (
            'missing input: target',
            '"1, 2, 3, 4"',
            'Usage: please provide a list of sorted integers ("1, 4, 5, 11, 12") and the integer to find ("11")'
        ), (
            'missing input: list',
            '"" "5"',
            'Usage: please provide a list of sorted integers ("1, 4, 5, 11, 12") and the integer to find ("11")'
        ), (
            'out of order input',
            '"3, 5, 1, 2" "3"',
            'Usage: please provide a list of sorted integers ("1, 4, 5, 11, 12") and the integer to find ("11")'
        )
    ]
)

valid_permutations = (
    'description,in_params,expected', [
        (
            'sample input first true',
            '"1, 3, 5, 7" "1"',
            'true'
        ), (
            'sample input last true',
            '"1, 3, 5, 7" "7"',
            'true'
        ), (
            'sample input middle true',
            '"1, 3, 5, 7" "5"',
            'true'
        ), (
            'sample input one true',
            '"5" "5"',
            'true'
        ), (
            'sample input one false',
            '"5" "7"',
            'false'
        ), (
            'sample input many false',
            '"1, 3, 5, 6" "7"',
            'false'
        )
    ]
)

@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

These are the tests. You need to replace these tests with the one's you wrote. For example, the first invalid test looks like this:

(
            'no input',
            None,
            'Usage: please provide a list of sorted integers ("1, 4, 5, 11, 12") and the integer to find ("11")'
)

You can replace it with this:

(
            'no input',
            None,
            'Usage: please input a number'
)

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 7, 2020

I have finished editing the test file. You may check and let me know what else I need to do please.

Copy link
Member

@jrg94 jrg94 left a comment

Choose a reason for hiding this comment

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

Looks good so far!

test/projects/test_palindromic_number.py Outdated Show resolved Hide resolved
test/projects/test_palindromic_number.py Outdated Show resolved Hide resolved
test/projects/test_palindromic_number.py Outdated Show resolved Hide resolved
test/projects/test_palindromic_number.py Outdated Show resolved Hide resolved
@jrg94
Copy link
Member

jrg94 commented Oct 7, 2020

Co-authored-by: Jeremy Grifski <jeremy.grifski@gmail.com>
@anohene1
Copy link
Contributor Author

anohene1 commented Oct 8, 2020

I have made both updates

Copy link
Member

@jrg94 jrg94 left a comment

Choose a reason for hiding this comment

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

Looks good! The only issue is the program you wrote will fail your own tests. We don't have a way to prove that just yet because someone is currently adding Kotlin testing.

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 8, 2020

Okay. So what do I do now?

@jrg94
Copy link
Member

jrg94 commented Oct 8, 2020

Update the Kotlin script to match your tests. Then, we're stuck waiting on #2186. Though, I'm not against merging this and letting that pull request solve the issue.

@anohene1
Copy link
Contributor Author

anohene1 commented Oct 8, 2020

I have fixed that.

Copy link
Member

@jrg94 jrg94 left a comment

Choose a reason for hiding this comment

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

I'm not sure your program matches your tests, but we'll go ahead and merge it for now. When Kotlin testing is implemented, we'll clean things up.

@jrg94 jrg94 merged commit 923c20f into TheRenegadeCoder:master Oct 8, 2020
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 needs docs New code that isn't documented here: https://sampleprograms.io/projects/
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants