Repository navigation
refactor: architecture restructure - eliminate Hungarian notation, reorganize directories, remove Factory - #4
Merged
Conversation
消除匈牙利命名法,统一为 Qt 风格语义化命名 (成员保留 m_ 前缀): - m_p/m_sp/m_up 指针前缀 → 去除类型编码 - m_n/m_i/m_ui 整型前缀 → 去除类型编码 - m_b 布尔前缀 → 去除类型编码 - m_str 字符串前缀 → 去除类型编码 - ms_ 静态成员 → s_ - NetworkRequestUtility → NetworkRequestUtils (文件 + 类) - NamEntry → ThreadNamEntry - 局部变量/参数: uiId→requestId, iMax→maxConcurrent 等 source/ 目录重组为 4 个子目录: - core/ — 核心基础设施 (manager, pool, registry, runnable, qtcompat) - requests/ — 请求类型 (common, download, upload, MT-download) - features/ — 功能扩展 (utils, environment, oauth2, collection, postman) - infra/ — 底层组件 (mmap, cookiejar, strategies) 移除废弃的 NetworkRequestFactory,调用点改为 Registry。 同步更新 CMakeLists.txt, QMake .pro, 测试代码, AGENTS.md, 设计文档。
wangrh0909
previously approved these changes
Aug 7, 2026
voidvec
commented
Aug 7, 2026
voidvec
left a comment
Owner
Author
There was a problem hiding this comment.
评审总结
重构整体质量良好,三项核心任务(命名消除、目录重组、Factory 移除)完成得很好。
必须修复 (2)
- maxRedirections copy-paste bug — source/core/networkrequestmanager.cpp:644 参数名应改为 maxConcurrent
- #include Qt 头文件应使用尖括号 — source/requests/networkcommonrequest.cpp:17-18 的 #include "QThread" 和 #include "QHttpMultiPart" 应改为 #include 和 #include
详细评审报告见 .codebuddy/pr-review-report.md
voidvec
commented
Aug 7, 2026
| QMutexLocker locker(&m_mutex); | ||
| m_mapRunnable.insert(r->requestId(), r); | ||
| m_runnableMap.insert(r->requestId(), r); | ||
| return true; |
Owner
Author
There was a problem hiding this comment.
[CRITICAL] Copy-paste bug: maxRedirections → maxConcurrent
// 声明 (第 71 行):
bool setMaxThreadCount(int maxConcurrent);
// 私有实现 (第 644 行) — BUG:
bool NetworkRequestManagerPrivate::setMaxThreadCount(int maxRedirections)参数名 maxRedirections(最大重定向次数)是错误的 — 这个函数的职责是设置线程池的最大线程数。这是一个明显的复制粘贴错误,应改为 maxConcurrent 与声明和 public 接口保持一致。
Suggested change
| return true; | |
| bool NetworkRequestManagerPrivate::setMaxThreadCount(int maxConcurrent) |
voidvec
commented
Aug 7, 2026
| m_nBytesSent = 0; | ||
| m_bytesReceived = 0; | ||
| m_bytesSent = 0; | ||
|
|
Owner
Author
There was a problem hiding this comment.
[MAJOR] Qt 头文件应使用 <>,第 18 行同样问题
项目中其他所有 Qt 头文件均使用 #include <QThread> 格式,此处使用引号 "" 违反编码规范。
Suggested change
| #include <QThread> | |
| #include <QHttpMultiPart> |
wangrh0909
previously approved these changes
Aug 9, 2026
This file contains hidden or 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
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.
Overview
Comprehensive architecture refactoring in three areas:
1. Eliminate Hungarian notation
Unified to Qt-style semantic naming (members keep m_ prefix):
2. Directory restructuring
source/ reorganized into 4 subdirectories:
3. Remove deprecated NetworkRequestFactory
Verification