Inconsistent insertion order for addMethodDeclaration fixed
What's changed?
In the previous code, the addMethodDeclaration method had a comparator that compared instances of Statement using UUIDs when they weren't instances of J.MethodDeclaration. This fallback to UUID comparison could lead to inconsistent placement of MethodDeclaration instances.
Here are the key changes made in the modified code:
Refined addMethodDeclaration Method:
Previously, the comparator used a fallback to UUID comparison for non-MethodDeclaration statements. This was changed to explicitly compare J.MethodDeclaration instances using the provided idealOrdering comparator. This ensures that new MethodDeclaration instances are consistently placed according to the ordering defined by idealOrdering, improving predictability. No Other Structural Changes:
The structure and organization of the classes and methods remain the same as in the previous code. The rest of the code continues to provide coordinate builders (JavaCoordinates) for various Java syntax elements (Statement, Expression, Annotation, etc.). These changes focus on enhancing the reliability and consistency of placing MethodDeclaration instances within Java code constructs, addressing the issue you encountered with intermittent test failures due to placement inconsistencies.
What's your motivation?
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
- Fixes #4203
Checklist
- [ ] I've added unit tests to cover both positive and negative cases
- [ ] I've read and applied the recipe conventions and best practices
- [ ] I've used the IntelliJ IDEA auto-formatter on affected files
@timtebeek please checkout this PR If any changes are required do ping me Thanks
Hi @Riyazul555 ; it looks like you've converted the ternary operator into an if statement, without changing the logic. I think it might be best to add a unit test to guarantee that we make the correct change, as I don't expect any improvement yet.
Ya I was just testing in another way Ok I will add it Thanks
Let's close this PR until such a test is added, just to make it clear this is not yet fixed. We can reopen the PR when a tests is added to the branch.