Minor fixes to serialization #451
Conversation
* Use array component type when serializing array items * Rename variables and methods * Use Iterable directly in simple JSON, as size is not needed
📝 WalkthroughWalkthroughThe pull request introduces modifications across multiple classes in the Joda Beans serialisation library, focusing on improving type handling, serialisation processes, and code clarity. Changes span three primary files: Changes
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonWriter.java (1)
186-191: Consider removing the unusedisRootflag
isRootis passed in but apparently is not used for any logic within this method. You could simplify this signature if no additional root-dependent logic will be added in future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/joda/beans/ser/bin/JodaBeanPackedBinWriter.java(1 hunks)src/main/java/org/joda/beans/ser/bin/JodaBeanStandardBinWriter.java(15 hunks)src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonWriter.java(10 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/main/java/org/joda/beans/ser/bin/JodaBeanStandardBinWriter.java (2)
Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.
- Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
- Validate that immutable local variables are not annotated with
finalunless the variable is required for use in an inner class. - Favour use of
varkeyword for type declarations.varmay also be used when the value is a castnull. - Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
- Propose changes that only use the Java 21 API, not the API of Guava.
- The pattern matching
instanceofexpression safely handlesnull, returningfalse.
Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.
- In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
The public Java API is defined as public and protected methods on public classes, plus the filemodule-info.java.
Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
Changes to be reported on include:- New or removed methods in the public Java API
- Changes to method return types or parameter types in the public Java API
- Changes to method behaviour in the public Java API that might affect consumers
- This project uses
System.out.printlninstead of logging - This project tends to prefer
forloops to streams for performance reasons, however either form is acceptable.
Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonWriter.java (2)
Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.
- Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
- Validate that immutable local variables are not annotated with
finalunless the variable is required for use in an inner class. - Favour use of
varkeyword for type declarations.varmay also be used when the value is a castnull. - Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
- Propose changes that only use the Java 21 API, not the API of Guava.
- The pattern matching
instanceofexpression safely handlesnull, returningfalse.
Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.
- In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
The public Java API is defined as public and protected methods on public classes, plus the filemodule-info.java.
Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
Changes to be reported on include:- New or removed methods in the public Java API
- Changes to method return types or parameter types in the public Java API
- Changes to method behaviour in the public Java API that might affect consumers
- This project uses
System.out.printlninstead of logging - This project tends to prefer
forloops to streams for performance reasons, however either form is acceptable.
Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/ser/bin/JodaBeanPackedBinWriter.java (2)
Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.
- Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
- Validate that immutable local variables are not annotated with
finalunless the variable is required for use in an inner class. - Favour use of
varkeyword for type declarations.varmay also be used when the value is a castnull. - Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
- Propose changes that only use the Java 21 API, not the API of Guava.
- The pattern matching
instanceofexpression safely handlesnull, returningfalse.
Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.
- In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
The public Java API is defined as public and protected methods on public classes, plus the filemodule-info.java.
Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
Changes to be reported on include:- New or removed methods in the public Java API
- Changes to method return types or parameter types in the public Java API
- Changes to method behaviour in the public Java API that might affect consumers
- This project uses
System.out.printlninstead of logging - This project tends to prefer
forloops to streams for performance reasons, however either form is acceptable.
Do not make suggestions to change between streams and for loops or vice versa.
🔇 Additional comments (17)
src/main/java/org/joda/beans/ser/json/JodaBeanSimpleJsonWriter.java (10)
68-68: Method reference usage is correct
The method reference writeBeanMaybeSimple looks consistent and typesafe.
160-160: Check appropriateness of using ResolvedType.OBJECT
Invoking writeBeanMaybeSimple with ResolvedType.OBJECT might be too general if you already know the concrete bean type. Confirm that this will not skip potential optimisations for specific bean classes.
176-177: Renamed method clarifies intent
Changing the name to writeBeanMaybeSimple indicates a two-step process (checking whether conversion is possible before falling back). The logic appears understandable.
182-182: Delegation to writeBean
Delegating to writeBean appears sensible, with the fallback path triggered when Joda-Convert is unavailable.
193-194: Property writing logic
writeBeanProperties uses an internal iteration of meta-properties and respects settings.isSerialized(...). This aligns with existing patterns.
225-225: Correct handling of array elements
Passing the exact array component type to handler.handle ensures each element is correctly serialised.
266-290: Map entries serialisation
This logic ensures null keys raise an exception, thus preventing malformed JSON. The approach is consistent with typical JSON constraints.
341-341: Method reference usage
Returning (JsonHandler<Collection<?>>) BaseJsonHandlers::writeCollection leverages the existing collection writer neatly.
351-351: Switched to Iterable<?>
Accepting any Iterable<?> broadens this function’s usage. This increases flexibility without affecting correctness.
369-370: Unified map serialisation call
Directly calling writer.writeMapEntries keeps logic centralised, aiding maintainability.
src/main/java/org/joda/beans/ser/bin/JodaBeanStandardBinWriter.java (6)
125-125: Parameter passing
Forwarding includeRootType to writeBean ensures consistent feature usage and simplifies future feature toggles.
151-151: Improved parameter name
Renaming isRoot to isRootAndInclRootType clarifies the combined logic behind the parameter.
172-172: Explicit naming for typeStr
Using typeStr clarifies that SerTypeMapper.encodeType results in a string, aiding readability.
173-173: Conditional check
Gatekeeping the type encoding under isRootAndInclRootType is unobtrusive and straightforward.
177-177: Extension string for bean type
Serialising the bean type as an extension string is consistent with the existing message pack approach.
261-261: Renaming helper method
Moving from writeMetaPropertyReference to writeMetaType better reflects this method’s role in encoding type metadata.
src/main/java/org/joda/beans/ser/bin/JodaBeanPackedBinWriter.java (1)
539-545: Refined type resolution for primitive arrays
Lines 539-545 now ensure each array element is passed to the correct handler with an appropriately constructed ResolvedType. This improves accuracy and consistency in the serialisation of primitive arrays.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor