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

Bug on complex-numbers test #1013

Closed
AfterRace opened this issue Sep 21, 2024 · 4 comments
Closed

Bug on complex-numbers test #1013

AfterRace opened this issue Sep 21, 2024 · 4 comments

Comments

@AfterRace
Copy link

All tests for the function c_abs() are against the same result 5.0.
So the following code pass all the tests.

double c_abs(complex_t x)
{
  (void)x;
  return 5.0;
}

I'd like to fix this bug, in order to send my first pull-request if someone explain me the process to do.

@AfterRace
Copy link
Author

Furthermore the fact that all the functions are badly defined in complex_number.c files, this is not the right way to implement TDD.
If it possible I would remove them.

@siebenschlaefer
Copy link
Contributor

You're right, the five tests for the function c_abs() all expect the result 5.0. To me it looks like the authors did that on purpose to demonstrate how different real and imaginary parts can result in the same absolute value. But I understand why that might not be ideal.

Also, the tests here at Exercism are not meant to be exhaustive. They should guide the student and catch some common bugs. They are not intended to catch each and every possible bug. IMHO a solution where c_abs() returns the fixed value 5.0 is not trying to solve the exercise, it tries to exploit the tests.

The description and the tests come from the language-agnostic problem-specification repository. If you want to propose additional tests, please do that in the forum so that everybody can participate in the discussion.


Furthermore the fact that all the functions are badly defined in complex_number.c files, this is not the right way to implement TDD.

I'm sorry I do not understand. Real TDD would be hard to do because the students would have to write their own tests (in advance). Exercism's approach is rather "TDD-adjacent": If you want you can enable each tests separately, so that you can focus on one test at a time.

There have been discussions in the past whether we should define some stubs in the .c file or declare functions in the .h file. Some argued that the students will have to read and understand the tests, so they can be expected to be able to extract the names and signatures of the functions from the tests. Others argued that this does not provide any additional value for the students and that most tracks here at Exercism have those stubs.

@wolf99
Copy link
Contributor

wolf99 commented Sep 21, 2024

Beat me to it by moments @siebenschlaefer , but your answer was better! 😀

@AfterRace
Copy link
Author

@wolf99 I get your point, I closed the issue because no action items are needed

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

No branches or pull requests

3 participants