Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve folder list index handling #2470

Merged
merged 2 commits into from
Dec 6, 2024
Merged

Conversation

wyu71
Copy link
Contributor

@wyu71 wyu71 commented Dec 6, 2024

  • Store folder index in item's UserRole data instead of relying on row number
  • Fix potential mismatch between visible items and data source
  • Optimize variable initialization order

Log: improve folder list index handling
Bug: https://pms.uniontech.com/bug-view-289995.html

Copy link
Contributor

@Johnson-zs Johnson-zs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没关联PMS?

- Store folder index in item's UserRole data instead of relying on row number
- Fix potential mismatch between visible items and data source
- Optimize variable initialization order

Log: improve folder list index handling
Bug: https://pms.uniontech.com/bug-view-289995.html
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码可读性

    • clicked函数中,将index.row()替换为index.data(Qt::UserRole).toInt()可能会改变原有的逻辑,需要确认这一改动是否符合预期。如果Qt::UserRole数据类型是整数,那么这个改动是合理的。如果不是,则需要恢复原来的代码。
  2. 性能优化

    • setFolderList函数中,使用for (auto data : datas)循环遍历datas列表,这可能会导致不必要的临时对象创建。建议使用for (int i = 0; i < datas.size(); ++i)循环,这样可以避免临时对象的创建。
  3. 代码重复

    • setFolderList函数中,isShowedHiddenFiles的赋值逻辑可以提取出来,避免重复代码。
  4. 资源管理

    • setFolderList函数中,创建的QStandardItem对象没有显示的删除操作,可能会导致内存泄漏。建议在适当的位置添加删除操作,或者使用智能指针来管理这些对象的生命周期。
  5. 错误处理

    • clicked函数中,如果index.data(Qt::UserRole).toInt()返回的值不是有效的行号,可能会导致数组越界。应该添加适当的错误处理逻辑,例如检查返回的行号是否在有效范围内。
  6. 注释和文档

    • 代码中没有足够的注释来解释clicked函数和setFolderList函数的逻辑,特别是对于一些关键步骤和变量的用途。建议添加更多的注释来提高代码的可读性和可维护性。

综上所述,建议对代码进行重构,以提高性能、可读性和安全性,并确保逻辑的正确性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs, wyu71

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Johnson-zs
Copy link
Contributor

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Dec 6, 2024

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 782d9d7 into linuxdeepin:master Dec 6, 2024
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants