Conversation
…el.git into copying
…delCopyTest concrete
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds copy functionality to the JCodeModel library, enabling deep copying of code models for testing and manipulation purposes. The implementation uses Java serialization to create independent copies of JCodeModel instances.
Key changes:
- Added a
copy()method to JCodeModel that uses serialization for deep copying - Implemented Serializable interface across multiple classes to support the copying mechanism
- Added comprehensive test infrastructure to verify copy functionality works correctly
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| JCodeModel.java | Added copy() and copySerial() methods for deep copying using serialization |
| ModelCopyTest.java | New test class that validates copy functionality with multiple modification scenarios |
| StringCodeWriter.java | New utility class for converting JCodeModel to string representation for testing |
| FSName.java | Made class implement Serializable and added missing @OverRide annotation |
| ClassNameComparator.java | Made class implement Serializable and added missing @OverRide annotation |
| JVar.java | Added getter for annotations and missing @OverRide annotations |
| JTypeVarClass.java | Added getter method and removed unnecessary braces |
| JResourceDir.java | Made class implement Serializable |
| JReferencedClass.java | Added getter method and missing @OverRide annotations |
| AbstractJAnnotationValueOwned.java | Added getter method and missing @OverRide annotations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
| catch (IOException | ClassNotFoundException e) | ||
| { | ||
| throw new UnsupportedOperationException ("catch this", e); |
There was a problem hiding this comment.
The error message "catch this" is unclear and unhelpful. Consider using a more descriptive message that explains the serialization failure, such as "Failed to copy JCodeModel through serialization".
| throw new UnsupportedOperationException ("catch this", e); | |
| throw new UnsupportedOperationException ("Failed to copy JCodeModel through serialization", e); |
| } | ||
| catch (IOException e) | ||
| { | ||
| throw new UnsupportedOperationException ("catch this", e); |
There was a problem hiding this comment.
The error message "catch this" is unclear and unhelpful. Consider using a more descriptive message that explains the code writing failure.
| throw new UnsupportedOperationException ("catch this", e); | |
| throw new UnsupportedOperationException ("Failed to write code model to string", e); |
|
@phax I think the correction is not good, throw exception without a message would be better. throw new UnsupportedOperationException (e); |
|
That's an error I had in my templates for exceptions, copied from the switch template which had a more explicit throw new UnsupportedOperationException ("undefined case "+c, e);The issue is that rethrowing the exception with a message can lead to unreadable stack traces due to the error message not being the correct one. |
|
@glelouet okay, point taken. Thanks for your responsiveness after all these years ;-) |
#94
some changes are formater-related.
basically add a copy() method in the JCM to copy itself. Also a test class for this new method. plus a few fields that are given accessors, and classes that are made implement serializable.