Binary heap #5084
Binary heap #5084
Conversation
🦋 Changeset detectedLatest commit: c083d79 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
contracts/utils/Comparators.sol
Outdated
There was a problem hiding this comment.
I like the idea of having comparators so that the heap is customizable. However, this library feels odd. Is this something we see value in providing on its own file? I'd rather keep it undocumented and inside Heap.sol
| @@ -22,6 +22,7 @@ import {ERC165} from "../utils/introspection/ERC165.sol"; | |||
| import {ERC165Checker} from "../utils/introspection/ERC165Checker.sol"; | |||
| import {ERC1967Utils} from "../proxy/ERC1967/ERC1967Utils.sol"; | |||
| import {ERC721Holder} from "../token/ERC721/utils/ERC721Holder.sol"; | |||
| import {Heap} from "../utils/structs/Heap.sol"; | |||
There was a problem hiding this comment.
Why not adding Comparators here?
| import {Heap} from "../utils/structs/Heap.sol"; | |
| import {Heap} from "../utils/structs/Heap.sol"; | |
| import {Comparators} from "../utils/Comparators.sol"; |
There was a problem hiding this comment.
It was overlooked, and since there are currently no tests for the Comparators library, it missing did not triger any issue.
Lets start by discussing weither we want the comparator library or not ... then we can add tests and solve that
| * A Heap is represented as an array of Node objects. In this array we store two overlapping structures: | ||
| * - A tree structure, where index 0 is the root, and for each index i, the children are 2*i+1 and 2*i+2. | ||
| * For each index in this tree we have the \`index\` pointer that gives the position of the corresponding value. | ||
| * - An array of values (payload). At each index we store a ${valueType} \`value\` and \`lookup\`, the index of the node | ||
| * that points to this value. |
There was a problem hiding this comment.
| * A Heap is represented as an array of Node objects. In this array we store two overlapping structures: | |
| * - A tree structure, where index 0 is the root, and for each index i, the children are 2*i+1 and 2*i+2. | |
| * For each index in this tree we have the \`index\` pointer that gives the position of the corresponding value. | |
| * - An array of values (payload). At each index we store a ${valueType} \`value\` and \`lookup\`, the index of the node | |
| * that points to this value. | |
| * A Heap is represented by an array of Node objects where two different data structures are represented: | |
| * | |
| * * A tree where the first element (i.e. at index 0) is the root and children are located at \`2*i+i\` and \`2*i+2\` for each index \`i\`. | |
| * * A list of payload values where each index contains a ${valueType} \`value\` and a \`lookup\` with its index in the tree. | |
| * | |
| * Every tree node includes an \`index\` pointer that indicates the position of its corresponding value within the list of payloads. |
There was a problem hiding this comment.
I feel this block should be in the top NatSpec and not in the struct docs
There was a problem hiding this comment.
lets discuss that part on our call
Co-authored-by: Ernesto García <[email protected]>
Fixes #3410
Alternative to #5076
This PR uses names from https://en.wikipedia.org/wiki/Heap_(data_structure)#Operations
For the inspection part, we use
lengthinstead ofsizefor consistencty with all other structures in theutils/structfolder.PR Checklist
npx changeset add)