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 pop-count (#940) #944

Merged
merged 5 commits into from
Jan 28, 2024
Merged

Add pop-count (#940) #944

merged 5 commits into from
Jan 28, 2024

Conversation

ahans
Copy link
Contributor

@ahans ahans commented Jan 27, 2024

Implements one of the missing exercises from #940.

config.json Outdated
@@ -2,14 +2,14 @@
"language": "C",
"slug": "c",
"active": true,
"blurb": "C is a small, general-purpose, imperative programming language with a static type system, scopes, and structures. It's typically used as an alternative to assembly programming, such as in operating systems.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as for the other PR applies: This was modified by configlet create. Let me know if I should revert it or put it into a different PR.

@ryanplusplus ryanplusplus mentioned this pull request Jan 27, 2024
12 tasks
Copy link
Contributor

@siebenschlaefer siebenschlaefer left a comment

Choose a reason for hiding this comment

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

Personally I would prefer an unsigned type for the argument because the behavior of right-shifting a negative number is implementation-defined.
But that's probably a subjective choice to some degree.


static void test_1_eggs(void)
{
const int expected = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

In the tests for almost all other exercises there is a line TEST_IGNORE(); (except in the first test), with an explaining comment in the second test.
see https://github.com/exercism/c/blob/main/exercises/practice/grains/test_grains.c#L19
That allows the tests to be enabled one by one so that the student can focus on one test at a time, similarly to Test-Driven Development.

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 was wondering about all of those TEST_IGNORE(), the explanation makes sense. Adapted it now.


int main(void)
{
UnityBegin("test_prime_factors.c");
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be "test_pop_count.c"

@ryanplusplus
Copy link
Member

Personally I would prefer an unsigned type for the argument because the behavior of right-shifting a negative number is implementation-defined. But that's probably a subjective choice to some degree.

I agree -- it should be unsigned.

@ahans
Copy link
Contributor Author

ahans commented Jan 28, 2024

Personally I would prefer an unsigned type for the argument because the behavior of right-shifting a negative number is implementation-defined. But that's probably a subjective choice to some degree.

If you pass in only positive numbers, it doesn't make a difference. But you're right of course, unsigned is cleaner here. Adapted it now.

@ahans
Copy link
Contributor Author

ahans commented Jan 28, 2024

I hope I addressed all comments, would be great if you could have another look!

bin/run-tests Outdated Show resolved Hide resolved
exercises/practice/pop-count/test_pop_count.c Outdated Show resolved Hide resolved
@ryanplusplus ryanplusplus merged commit 385e057 into exercism:main Jan 28, 2024
4 checks passed
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.

3 participants