Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Jun 11, 2025

调整 RoleGroupModel 以及 RoleCombineModel 便于后续维护.

(这是迁移自图标拆分分支的变更)

Log:

Summary by Sourcery

Refactor role handling in proxy models by splitting and caching roleNames in RoleCombineModel for easier maintenance, and add a safety check in RoleGroupModel::rowCount.

Enhancements:

  • Refactor RoleCombineModel to separate roleNames creation into createRoleNames and cache results in m_roleNames
  • Update RoleCombineModel’s minor role mapping to use the cached role names
  • Add a null-check guard in RoleGroupModel::rowCount to handle unset source models

调整 RoleGroupModel 以及 RoleCombineModel 便于后续维护.

(这是迁移自图标拆分分支的变更)

Log:
@BLumia BLumia requested review from robertkill and yixinshark June 11, 2025 11:49
@deepin-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@BLumia BLumia marked this pull request as ready for review June 11, 2025 11:49
@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • createRoleNames 函数中的 lastRole 变量可能未初始化,应该确保在使用之前初始化。
  • RoleCombineModel 类中的 m_roleNames 成员变量在 createRoleNames 函数中被赋值,但没有检查 sourceModel() 是否为空。
  • RoleGroupModel 类中的 rowCount 函数在调用 sourceModel() 之前应该检查 sourceModel() 是否为空。

是否建议立即修改:

@sourcery-ai
Copy link

sourcery-ai bot commented Jun 11, 2025

Reviewer's Guide

Refactors RoleCombineModel by extracting and caching its roleNames generation into a dedicated method for clarity and maintenance, and adds a safety null-check in RoleGroupModel::rowCount to prevent errors when the source model is unset.

Sequence Diagram: RoleCombineModel Role Name Generation, Caching, and Access

sequenceDiagram
    participant ClientCode as "Client Code"
    participant Constructor as "RoleCombineModel(...)"
    participant RCM_Instance as "RoleCombineModel Instance"
    participant CreateRN_Method as "RCM_Instance.createRoleNames()"
    participant RoleNames_Method as "RCM_Instance.roleNames()"

    Note over ClientCode, RoleNames_Method: Illustrates refactoring of role name generation and access in RoleCombineModel

    ClientCode ->> Constructor: new RoleCombineModel(major, minor)
    activate Constructor
        Constructor ->> CreateRN_Method: Call to generate and combine role names
        activate CreateRN_Method
            Note right of CreateRN_Method: Internally fetches role names from sourceModel and m_minor
            CreateRN_Method -->> Constructor: combined_role_names
        deactivate CreateRN_Method
        Constructor ->> RCM_Instance: m_roleNames = combined_role_names (Cache populated)
        Note over Constructor, RCM_Instance: m_minorRolesMap is then populated using m_roleNames.key() (cached)
    deactivate Constructor

    ClientCode ->> RoleNames_Method: getRoleNames()
    activate RoleNames_Method
        RoleNames_Method ->> RCM_Instance: Access m_roleNames
        RCM_Instance -->> RoleNames_Method: Returns cached m_roleNames
    deactivate RoleNames_Method
    RoleNames_Method -->> ClientCode: cached_role_names
Loading

Class Diagram: RoleCombineModel Refactoring for Role Name Caching

classDiagram
  class RoleCombineModel {
    +RoleCombineModel(QAbstractItemModel* major, QAbstractItemModel* minor, QObject *parent) // Logic modified for caching
    +roleNames() const : QHash<int, QByteArray> // Now returns cached m_roleNames
    -createRoleNames() const : QHash<int, QByteArray> // New: Encapsulates role name generation
    -m_roleNames: QHash<int, QByteArray> // New: Cache for role names
    -m_minor: QAbstractItemModel* // Existing relevant member
    -m_minorRolesMap: QHash<int, int> // Existing relevant member, population logic changed
  }
  RoleCombineModel --|> QAbstractProxyModel
Loading

Class Diagram: RoleGroupModel Update to rowCount Method

classDiagram
  class RoleGroupModel {
    +rowCount(const QModelIndex &parent) const : int // Logic modified: Added null check for sourceModel()
    +setSourceModel(QAbstractItemModel *model) // Existing method, provides context
  }
  RoleGroupModel --|> QAbstractProxyModel
Loading

File-Level Changes

Change Details Files
Extract and cache roleNames in RoleCombineModel
  • Extracted roleNames assembly into createRoleNames()
  • Introduced m_roleNames member to store computed role names
  • Modified roleNames() to return the cached m_roleNames
  • Updated minorRolesMap insertion to reference m_roleNames
  • Declared createRoleNames() in the header
panels/dock/taskmanager/rolecombinemodel.cpp
panels/dock/taskmanager/rolecombinemodel.h
Add null-check guard in RoleGroupModel::rowCount
  • Return 0 when sourceModel() is unset before proceeding
panels/dock/taskmanager/rolegroupmodel.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @BLumia - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@BLumia BLumia requested a review from 18202781743 June 12, 2025 05:30
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, BLumia, yixinshark

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

Details 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

@BLumia BLumia merged commit 432e419 into linuxdeepin:master Jun 12, 2025
7 of 10 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.

4 participants