Skip to content

Remove duplicated if checking in add_into_heap for cpu. - #49

Merged
hzhangxyz merged 1 commit into
mainfrom
dev/remove-duplicated-if-check
Jun 26, 2025
Merged

Remove duplicated if checking in add_into_heap for cpu.#49
hzhangxyz merged 1 commit into
mainfrom
dev/remove-duplicated-if-check

Conversation

@hzhangxyz

Copy link
Copy Markdown
Member

Description

Checklist:

Copilot AI review requested due to automatic review settings June 20, 2025 07:04

This comment was marked as outdated.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

The PR cleans up redundant checks and braces in the add_into_heap function to simplify its control flow.

  • Removed a duplicated empty if (compare(value, heap[index])) guard
  • Flattened nested conditional logic for child‐node handling
  • Eliminated an extra closing brace
Comments suppressed due to low confidence (1)

qmb/_hamiltonian_cpu.cpp:307

  • [nitpick] Consider adding unit tests for the scenario where only the right child is present (this branch) to ensure correct heap behavior.
                if (right_present) {

Comment thread qmb/_hamiltonian_cpu.cpp
@@ -261,91 +261,64 @@ void add_into_heap(T* heap, std::int64_t heap_size, const T& value) {
std::int64_t index = 0;
if (compare(value, heap[index])) {
} else {

Copilot AI Jun 24, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Use an early return instead of an empty if‐block to reduce nesting. For example:

if (compare(value, heap[index]))
    return;
Suggested change
} else {
return;
}

Copilot uses AI. Check for mistakes.
@hzhangxyz
hzhangxyz requested review from msg-bq and removed request for CuSO4Deposit June 24, 2025 11:49
@hzhangxyz

hzhangxyz commented Jun 25, 2025

Copy link
Copy Markdown
Member Author

@CuSO4Deposit 由于修改compare变量名为less,同时依赖此PR和一个已经merge进main的PR,我打算等这个PR merge了再改名字。

@msg-bq msg-bq left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM(小声感慨一下好深的嵌套,一行行对齐看了下)

@hzhangxyz
hzhangxyz merged commit 3c47308 into main Jun 26, 2025
2 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants