I’m currently working on the Complex Number exercise, which is part of the Java track, and would like to enhance the test coverage by adding new test cases. Specifically, I want to include test cases for operations involving multiplying by a factor and dividing by a divisor, rather than just testing with complex numbers.
Background
The functions for multiplying using a factor and dividing using a divisor are provided at the start of the code. However, I’ve noticed that the code passes the tests even if these functions return null, as they are not tested at all. This is concerning because it means that the functionality is not being validated properly.
Proposed Test Cases
I plan to add the following test cases to the existing JSON file:
Multiplication by Factor
Input: A complex number (e.g., 3 + 4i)
Factor: A numeric value (e.g., 2)
Expected Output: A new complex number representing the result of the multiplication (e.g., 6 + 8i)
Division by Divisor
Input: A complex number (e.g., 3 + 4i)
Divisor: A numeric value (e.g., 2)
Expected Output: A new complex number representing the result of the division (e.g., 1.5 + 2i)
Request for Guidance
I would appreciate any advice on how to properly structure these new test cases in the JSON file and how to integrate them into the existing test suite. Additionally, if there are best practices I should follow when making a Pull Request for these changes, I’d love to hear those as well.
What makes you think that the functionality isn’t validated properly? Do you have a specific example in mind?
Multiplying by a factor of 2 is the same as multiplying by the complex number 2 + 0*i.
Dividing by a factor of 2 is the same as dividing by the same complex number.
If these are allowed, we would also need to add cases where a complex number is added to, subtracted from, multiplied by, or divided by a number that has only a real or imaginary part.
However, there are already tests covering addition, subtraction, multiplication, and division with the most general cases of complex numbers—those that have both real and imaginary parts. Isn’t passing those tests enough to ensure proper functionality and coverage? Personally, I would add one more similar test case for each operation, simply because there is currently only one test case for each.
Thank you for your feedback and for engaging in this discussion!
I appreciate the work that has gone into developing the current tests, and I want to clarify my intention behind suggesting additional test cases.
When starting the exercise, we are provided with two versions of the multiply function: one that accepts a complex number and another that accepts a double. Similarly, we have the divide function in both forms. My concern is that while we have tests validating operations with complex numbers, there are currently no tests validating the functionality that specifically uses the double version of these functions.
For instance, while there are already test cases that validate multiplication and division with real numbers using the complex number class as the parameter (with the imaginary part set to 0), my concern is that we also have a multiply function that accepts a double. It’s important to include tests for this function as well to ensure comprehensive coverage.
My goal in proposing additional test cases is not to undermine the existing work but rather to enhance it by ensuring comprehensive coverage of all provided functions. As a student, I want to contribute back to Exercism, as I have greatly benefited from it. My aim is simply to help ensure that all parts of the implementation are thoroughly validated.
I hope this clears up any misunderstandings, and I appreciate your consideration of my suggestion to include these additional test cases.
I appreciate your willingness to add another similar test case for each functionality! I would be more than happy to take on this task as I have a good grasp of the concepts. However, I would need some guidance on how to access the relevant files and understand the workflow structure of Exercism. Once I have that information, I can create the necessary test cases without any problem. Thank you for your support!
It seems strange that there are versions that take the double, but no tests. The test data for the exercise is shared across tracks in the canonical data and there is already coverage for that (for both multiplication and division).
But, looking at the exercise’s test data (for the Java track) history (in its test.toml), the tests were left out for the Java track to focus on just two complex numbers. I’m not quite sure what happened since there are also methods that take the double (I’d expect the double versions to not be there if we aren’t going to have the tests).
Given this, I think we should either remove the double version of the methods or add the those test cases from the canonical data. If you’d like to open a PR to add the data, I think you’d just need to update the exercise’s test and the test.toml. You might need to also update the example solution.
@jagdishdrp Based on your replies, this does seem to be an issue specific to the Java track. Please continue this with @kahgoh or any other maintainer of the Java track. If the changes still don’t address your concerns and you want to propose updates that apply across all tracks, we can continue the discussion here or in a new thread.
Thank you for the clarification! Since other tracks do not have separate functions for multiplication and division of complex numbers with real numbers (using doubles), I agree that it would be more consistent to remove those methods from the exercise. The absence of corresponding tests in both the test.toml and test.java reinforces this decision.
Updating the example solution should address this, but do we also need to make changes in any other files to prevent the methods that accept doubles from being generated when we start the exercise? I want to ensure that we take the right steps for a clean implementation.
Thanks for your guidance!
Edit- After reviewing the files, I’ve realized that I’ll need to update both the example solution and the main solution, so I’ll proceed with making a PR. Out of curiosity, I was wondering how reputation points work for contributions like this. If my Exercism profile is linked with my GitHub handle, will points be credited automatically once the PR is accepted?
Yup, the methods should also be removed from the stub file. Also, yes, you’ll get reputation points once the PR is accepted if you link your GitHub account.
I’ll go ahead and work on the PR. Meanwhile, could you let me know how I can help improve the instructions writeup for this exercise, or any others in the future? I assume this involves writing in Markdown, but please correct me if I’m wrong, and I’d appreciate any tips on how to get started.
Edit: I have successfully located the instructions.md file in the docs folder of the Complex Number exercise in the Java track. However, I understand that changing and submitting a PR for this exercise will only update the instructions for this track. Should I focus solely on these changes, or do you have any suggestions for making updates that would apply across all tracks?
Thanks for raising the PR @jagdishdrp! I’ll take a look at the PR in a little while. In regards to the instructions.md, what is the proposed change?
instructions.md file is synced across tracks in the problem specifications and we generally encourage a discussion for changes there. However, tracks can also add track specific notes in an instructions.append.md.
I’m proposing the following changes to instructions.md to improve readability and compatibility, especially on platforms like GitHub where LaTeX syntax isn’t consistently rendered. Here’s the updated markdown:
This update doesn’t introduce new content; it primarily refines the formatting to make it more consistent and visually accessible across different platforms. I’m not suggesting that the current version is lacking in quality—it’s quite effective as-is. I simply wanted to offer these improvements as a way to give back to Exercism, keeping user experience in mind.
I hope this version reflects your suggestions, @tasx. Since the coloring should not appear in the actual instructions tab, I believe everything is in order!