feat: 支持 JS 对象和数组转 Map 和 Array #75
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
审核者指南 by Sourcery此拉取请求实现了将 JavaScript 对象和数组转换为 Java Map 和 Array 结构的支持。主要更改包括添加新方法以将 JSObject 和 JSArray 实例转换为它们的 Java 对应物，处理循环引用，并实现特定类型的转换逻辑。 序列图sequenceDiagram
participant Client
participant JSObject
participant ConvertToMap
participant JavaMap
Client->>JSObject: toMap()
JSObject->>ConvertToMap: convertToMap(this, new HashMap)
loop 对每个属性
ConvertToMap->>ConvertToMap: 检查循环引用
ConvertToMap->>ConvertToMap: 根据类型转换值
ConvertToMap->>JavaMap: 放置转换后的值
end
ConvertToMap-->>JSObject: 返回转换后的 map
JSObject-->>Client: 返回 Java Map
文件级更改
提示和命令与 Sourcery 互动
自定义您的体验访问您的仪表板以：
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request implements support for converting JavaScript objects and arrays to Java Map and Array structures. The main changes include adding new methods to convert JSObject and JSArray instances to their Java counterparts, handling circular references, and implementing type-specific conversion logic. Sequence DiagramsequenceDiagram
participant Client
participant JSObject
participant ConvertToMap
participant JavaMap
Client->>JSObject: toMap()
JSObject->>ConvertToMap: convertToMap(this, new HashMap)
loop For each property
ConvertToMap->>ConvertToMap: Check circular reference
ConvertToMap->>ConvertToMap: Convert value based on type
ConvertToMap->>JavaMap: Put converted value
end
ConvertToMap-->>JSObject: Return converted map
JSObject-->>Client: Return Java Map
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嗨 @HarlonWang - 我已经审查了你的更改 - 这里有一些反馈：
总体评论：
- 考虑改进内存管理，特别是对于嵌套结构。确保所有 JavaScript 对象在转换后都被正确释放。
- 统一不同类型（JSObject、JSArray、JSFunction）不支持转换的错误信息。
这是我在审查期间查看的内容
- 🟡 一般问题：发现 1 个问题
- 🟢 安全性：一切看起来都很好
- 🟢 测试：一切看起来都很好
- 🟡 复杂性：发现 1 个问题
- 🟢 文档：一切看起来都很好
帮助我变得更有用！请点击每条评论上的 👍 或 👎 告诉我它是否有帮助。
Original comment in English
Hey @HarlonWang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider improving memory management, especially for nested structures. Ensure all JavaScript objects are properly released after conversion.
- Standardize error messages for unsupported conversions across different types (JSObject, JSArray, JSFunction).
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
wrapper-java/src/main/java/com/whl/quickjs/wrapper/QuickJSObject.java
Outdated
Show resolved
Hide resolved
| throw new UnsupportedOperationException("Object types are not yet supported for conversion to array. You should use toMap."); | ||
| } | ||
|
|
||
| protected void convertToMap(Object target, Object map) { |
There was a problem hiding this comment.
issue (complexity): 考虑将 'convertToMap' 方法重构为更小、更专注的方法。
convertToMap 方法引入了显著的复杂性。虽然功能很重要，但我们建议以下改进以增强可读性和可维护性：
- 将
convertToMap方法分解为更小、更专注的私有方法：
private void convertToMap(Object target, Object map) {
if (handleCircularReference(target)) return;
if (target instanceof JSArray) {
handleArrayConversion((JSArray) target, map);
} else {
handleObjectConversion((JSObject) target, map);
}
}
private boolean handleCircularReference(Object target) {
long pointer = ((JSObject) target).getPointer();
if (circulars.contains(pointer)) return true;
circulars.add(pointer);
return false;
}
private void handleArrayConversion(JSArray array, Object map) {
// ... implementation ...
}
private void handleObjectConversion(JSObject object, Object map) {
// ... implementation ...
}- 封装循环引用处理：
不要使用受保护的circulars字段，而是在toMap中使用局部变量：
public HashMap<String, Object> toMap() {
HashMap<String, Object> objectMap = new HashMap<>();
ArrayList<Long> circulars = new ArrayList<>();
convertToMap(this, objectMap, circulars);
return objectMap;
}
private void convertToMap(Object target, Object map, ArrayList<Long> circulars) {
// 使用 circulars 参数而不是字段
}- 移除
toArray方法，因为它未实现并抛出异常。
这些更改将保留功能，同时减少复杂性并提高可维护性。
Original comment in English
issue (complexity): Consider refactoring the 'convertToMap' method into smaller, focused methods.
The convertToMap method introduces significant complexity. While the functionality is important, we suggest the following improvements to enhance readability and maintainability:
- Break down the
convertToMapmethod into smaller, focused private methods:
private void convertToMap(Object target, Object map) {
if (handleCircularReference(target)) return;
if (target instanceof JSArray) {
handleArrayConversion((JSArray) target, map);
} else {
handleObjectConversion((JSObject) target, map);
}
}
private boolean handleCircularReference(Object target) {
long pointer = ((JSObject) target).getPointer();
if (circulars.contains(pointer)) return true;
circulars.add(pointer);
return false;
}
private void handleArrayConversion(JSArray array, Object map) {
// ... implementation ...
}
private void handleObjectConversion(JSObject object, Object map) {
// ... implementation ...
}- Encapsulate circular reference handling:
Instead of using a protectedcircularsfield, use a local variable intoMap:
public HashMap<String, Object> toMap() {
HashMap<String, Object> objectMap = new HashMap<>();
ArrayList<Long> circulars = new ArrayList<>();
convertToMap(this, objectMap, circulars);
return objectMap;
}
private void convertToMap(Object target, Object map, ArrayList<Long> circulars) {
// Use circulars parameter instead of field
}- Remove the
toArraymethod as it's not implemented and throws an exception.
These changes will retain the functionality while reducing complexity and improving maintainability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#65
Sourcery 的总结
在 QuickJS 包装器中添加将 JavaScript 对象和数组转换为 Java HashMap 和 ArrayList 的功能，并支持处理循环引用。实现相应的测试以确保正确的转换行为。
新功能：
增强功能：
测试：
Original summary in English
Summary by Sourcery
Add functionality to convert JavaScript objects and arrays to Java HashMap and ArrayList in the QuickJS wrapper, with support for handling circular references. Implement corresponding tests to ensure correct conversion behavior.
New Features:
Enhancements:
Tests: