Refactored data clumps with the help of LLMs (research project) #1962
Conversation
There was a problem hiding this comment.
Sorry but I am not convinced that these type of changes make the code more readable or maintainable. So I am against integrating such a code also after addressing my findings here, because the code is not better than the original and also just removed minor code duplication by introducing new code complexity.
Also due to changing the immutable classes to mutable ones, the change functionally breaks the code for concurrent usages, see chapter Safe Publication in Concurrency in Practice for details.
| private final boolean useObjenesis; | ||
| private final IMockMakerSettings mockMakerSettings; | ||
| private MockSettings mockSettings = new MockSettings(null, null, null, null, false, false); |
There was a problem hiding this comment.
This makes the object mutable due the non immutable object and non final field.
| private final SpecificationAttachable mockInterceptor; | ||
|
|
||
| private Specification specification; | ||
| private MockSettings mockSettings = new MockSettings(null, null, null, null, false, false); |
There was a problem hiding this comment.
This makes the object mutable due the non immutable object and non final field.
| this.mockSettings.setName(name); | ||
| this.mockSettings.setType(type); | ||
| this.mockSettings.setInstance(instance); | ||
| this.mockSettings.setVerified(verified); | ||
| this.mockSettings.setGlobal(global); | ||
| this.mockSettings.setDefaultResponse(defaultResponse); |
There was a problem hiding this comment.
Also the code change is not better readable then the other one, also the removed "code duplication" increases the overall code size => This does not make the code more maintainable.
| @@ -0,0 +1,72 @@ | |||
| package org.spockframework.mock.runtime; | |||
There was a problem hiding this comment.
This class does not adhere the formatting rule, code positions e.g. fields before constructor, qualified names instead of imports etc.
Nor is it Immutable, nor has a toString (for debugging purpose) + equal+ hashcode for POJO, etc.
So this new class does not make the original code more readable nor is this class maintainable.
| @@ -0,0 +1,72 @@ | |||
| package org.spockframework.mock.runtime; | |||
| public class MockSettings{ | |||
| private java.lang.String name; | |||
There was a problem hiding this comment.
Nullable annotations are missing, or null checks in the constructor/setter missing.
|
Thank you very much for the feedback. It is absolutely fine if you do not want these changes to be merged. You feedback is still very valuable and I appreciate your time |
Hello maintainers,
I am conducting a master thesis project focused on enhancing code quality through automated refactoring of data clumps, assisted by Large Language Models (LLMs).
Data clump definition
A data clump exists if
See also the following UML diagram as an example
I believe these refactoring can contribute to the project by reducing complexity and enhancing readability of your source code.
Pursuant to the EU AI Act, I fully disclose the use of LLMs in generating these refactorings, emphasizing that all changes have undergone human review for quality assurance.
Even if you decide not to integrate my changes to your codebase (which is perfectly fine), I ask you to fill out a feedback survey, which will be scientifically evaluated to determine the acceptance of AI-supported refactorings. You can find the feedback survey under https://campus.lamapoll.de/Data-clump-refactoring/en
Thank you for considering my contribution. I look forward to your feedback. If you have any other questions or comments, feel free to write a comment, or email me under tschoemaker@uni-osnabrueck.de .
Best regards,
Timo Schoemaker
Department of Computer Science
University of Osnabrück