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

[perfect-numbers] backticks on math and removed parens #2319

Merged
merged 12 commits into from
Dec 13, 2023

Conversation

habere-et-dispertire
Copy link
Contributor

Thanks @MatthijsBlom 🌷
See : #2318 (comment)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Hello. Thanks for opening a PR on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed.

That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there.


Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.

@github-actions github-actions bot closed this Sep 7, 2023
@habere-et-dispertire habere-et-dispertire changed the title backticks on math and removed parens [perfect-numbers] backticks on math and removed parens Sep 7, 2023
@kotp kotp reopened this Sep 7, 2023
@habere-et-dispertire habere-et-dispertire marked this pull request as ready for review September 8, 2023 00:30
@habere-et-dispertire habere-et-dispertire requested a review from a team as a code owner September 8, 2023 00:30
- 24 is an abundant number because (1 + 2 + 3 + 4 + 6 + 8 + 12) = 36
- **Deficient**: aliquot sum < number
- 8 is a deficient number because (1 + 2 + 4) = 7
- **Perfect**: when a `number` equals its `aliquot sum`
Copy link
Member

Choose a reason for hiding this comment

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

Should number and aliquot sum be quoted rather than code blocked?

Copy link
Member

Choose a reason for hiding this comment

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

I will take that as a no, then. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's perhaps easier to read wth the code blocks marking them as variables which also draws us to their earlier mention -- but I don't mind. It currently looks like


The aliquot sum is defined as the sum of the factors of a number not including the number itself.
For example, the aliquot sum of 15 is 1 + 3 + 5 = 9.

  • Perfect: when a number equals its aliquot sum
  • Abundant: when a number is less than its aliquot sum
  • Deficient: when a number is greater than its aliquot sum

Copy link
Member

@kotp kotp Sep 8, 2023

Choose a reason for hiding this comment

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

I thought we were describing the idea, rather than actual code. It seems that aloquot sum may or may not be an invocation of a function or method in some languages.

I definitely will not stand in the way of it though.

It could look like:


The "aliquot sum" is defined as the sum of factors of a number not including the number itself.
For example, the "aliquot sum" of 15 is 1 + 3 + 5 or 9.

  • Perfect: when number equals its "aliquot sum"
  • Abundant: when number less than its "aliquot sum"
  • Deficient: when number greater than its "aliquot sum"

Copy link
Member

Choose a reason for hiding this comment

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

The aliquot sum is defined as the sum of factors of a number not including the number itself. For example, the aliquot sum of 15 is 1 + 3 + 5 or 9.

  • Perfect: when number equals its aliquot sum
  • Abundant: when number less than its aliquot sum
  • Deficient: when number greater than its aliquot sum

—-

Italics to me emphasize the technical term aliquot sum but also distinct from other terms that are bolded, a different category of emphasis. Quote marks to me don’t convey that much emphasis compared to italics.

Copy link
Member

Choose a reason for hiding this comment

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

I would also prefer bolding it

Copy link
Member

Choose a reason for hiding this comment

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

The reason I kept the emphasis on each occurrence was because perfect, abundant, and deficient are also bolded in later occurrences. It’d make sense though to only emphasize the first occurrence so maybe that needs to fixed as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've turned them into links referencing the relevant heading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everyone happy with this ?

Copy link
Member

Choose a reason for hiding this comment

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

Wasn't terrible to start out with, but still looking much better now!

Taking contributor concerns into play I've also italicized only the defining instance of aliquot sum.
@kotp
Copy link
Member

kotp commented Sep 10, 2023

Just waiting on the code owner approval... looking good, though.

@senekor
Copy link
Contributor

senekor commented Dec 12, 2023

@ErikSchierboom ping 🙂

@kotp kotp merged commit 0f7a7f8 into exercism:main Dec 13, 2023
Sign up for free to 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.

7 participants