Skip to content

Ordinal numbers exercise - #351

Merged
neenjaw merged 7 commits into
exercism:mainfrom
codedge:ordinal-numbers
Mar 21, 2021
Merged

Ordinal numbers exercise#351
neenjaw merged 7 commits into
exercism:mainfrom
codedge:ordinal-numbers

Conversation

@codedge

@codedgecodedge commented Mar 20, 2021

Copy link
Copy Markdown
Contributor

Provide excercise for ordinal numbers.

https://en.wikipedia.org/wiki/Ordinal_numeral

@codedgecodedge changed the title Provide instruction, code and testsOrdinal numbers exerciseMar 20, 2021

@neenjawneenjaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey @codedge!!

Thanks for contributing to the PHP track! 💙 This exercise is a great addition to the track. I left some review comments to be addressed and then we'll get it merged in.

The website is in code-freeze right now, so it will not appear until v3 of Exercism is released (April-ish timeline at present). If you have any questions about my feedback, please ask here or in our slack.

Again, thanks for contributing! @djjohnsongeek and I are looking for active contributors to build out PHP for v3 too if your interested in doing more.

Comment threadexercises/practice/ordinal-number/.docs/instructions.md Outdated
Comment threadexercises/practice/ordinal-number/.docs/instructions.md Outdated
Comment threadexercises/practice/ordinal-number/.meta/example.php Outdated
Comment threadexercises/practice/ordinal-number/OrdinalNumber.php Outdated

function ordinalNumber(int $number): string
{
//

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This may break with other exercises, but what do you think about this throwing an exception? eg:

thrownew \BadFunctionCallException("Implement the ordinalNumber function");

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I like the idea but I would also love to suggest using proper type hinting. With giving an example like function toOrdinal(int $number): string the user can visually see the type hint. Additionally I suggest using strict_types.

<?phpdeclare(strict_types=1);
functionordinalNumber(int$number): string {
// Your code goes here
}

Any idea how to point to proper type hints in a different way?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

One more point:
If I do not suggest the name of the function, against which method do I test in the test class?
Does exercism enfore a particular name per exercise, like Write a function `toOrdinal` ... ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggesting the function name is good! Sorry if that was unclear. The file would look like:

<?phpfunctiontoOrdinal(int$number): string {
thrownew \BadFunctionCallException("Implement the toOrdinal function");
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As far as strict types, I'm not opposed to this idea, but I think we should establish a convention that add it to the whole track for consistency.

Would you be willing to open an issue and tag @petemcfarlane, @djjohnsongeek, @arueckauer, and myself to it?

@codedgecodedgeMar 21, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

No problem. I opened a new issue - #352

The BadFunctionCallException is added.

Comment threadexercises/practice/ordinal-number/OrdinalNumber.php Outdated
Comment threadexercises/practice/ordinal-number/OrdinalNumberTest.php Outdated
@codedge

Copy link
Copy Markdown
ContributorAuthor

@neenjaw I'd love to contribute more. I got some more exercises in the pipe 😉 Of course I can also help with improving the existing exercises.

I am currently doing the php exercises on the platform itself to get a complete overview.

Comment threadexercises/practice/ordinal-number/OrdinalNumberTest.php
Comment threadexercises/practice/ordinal-number/.meta/config.json Outdated
Comment threadexercises/practice/ordinal-number/.meta/config.json Outdated
Comment threadexercises/practice/ordinal-number/.meta/config.json Outdated
Comment threadexercises/practice/ordinal-number/.docs/instructions.md
@codedgecodedge mentioned this pull request Mar 21, 2021
Comment threadexercises/practice/ordinal-number/OrdinalNumber.php Outdated
Comment threadexercises/practice/ordinal-number/OrdinalNumberTest.php Outdated
Comment threadexercises/practice/ordinal-number/.meta/example.php Outdated
@neenjaw

Copy link
Copy Markdown
Collaborator

3 things left then let's get this merged!!

@neenjawneenjaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great!

Awesome work, @codedge!

@neenjaw
neenjaw merged commit a3ab7a0 into exercism:mainMar 21, 2021
@codedgecodedge mentioned this pull request Mar 24, 2021
Sign up for freeto 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.

2 participants

@codedge@neenjaw