Skip to content

Conversation

@17802723
Copy link
Contributor

No description provided.

@jetjinser jetjinser self-requested a review January 29, 2026 06:03
Copy link
Collaborator

@jetjinser jetjinser left a comment

Choose a reason for hiding this comment

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

后面的还没看,先改成用 hash-table 存 entry 吧

(comparator bag-element-comparator))

(define (check-bag obj)
(if (not (bag? obj)) (type-error "not a bag" obj)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(if (not (bag? obj)) (type-error "not a bag" obj)))
(when (not (bag? obj)) (type-error "not a bag" obj)))

Comment on lines 569 to 576
(define (bag-entry element count)
(cons element count))

(define (bag-entry-element entry)
(car entry))

(define (bag-entry-count entry)
(cdr entry))
Copy link
Collaborator

Choose a reason for hiding this comment

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

bag-entry 如果需要这些 proc 的话,也用 record,不要这么写,这里相当于手动构造 record 了。

但是没必要另外构造这个,entries 用一个 hash-table 就行。

另外如果这么写,也写得复杂了一些

Suggested change
(define (bag-entry element count)
(cons element count))
(define (bag-entry-element entry)
(car entry))
(define (bag-entry-count entry)
(cdr entry))
(define (bag-entry element count)
(cons element count))
(define bag-entry-element car)
(define bag-entry-count cdr)

Comment on lines 580 to 581
(if (not (and (exact-integer? count) (>= count 0)))
(type-error "bag-increment!" count))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(if (not (and (exact-integer? count) (>= count 0)))
(type-error "bag-increment!" count))
(unless (and (exact-integer? count) (>= count 0))
(type-error "bag-increment!" count))

(%make-bag entries comparator)
bag?
(entries bag-entries set-bag-entries!)
(comparator bag-element-comparator))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(comparator bag-element-comparator))
(comparator bag-comparator))

保持简单些吧,不然有可能会带来 bag-element 的 comparator field 的混淆。

Comment on lines 584 to 594
(let* ((eq (comparator-equality-predicate (bag-element-comparator bag)))
(entries (bag-entries bag)))
(let loop ((rest entries))
(cond
((null? rest)
(set-bag-entries! bag (cons (bag-entry element count) entries)))
(else
(let ((entry (car rest)))
(if (eq (bag-entry-element entry) element)
(set-cdr! entry (+ count (bag-entry-count entry)))
(loop (cdr rest)))))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

因为 entries 是个 alist,可以用 assq 实现,但更好的方式还是 hash-table-xxx 的操作

Comment on lines 597 to 618
(define (bag-decrement! bag element count)
(check-bag bag)
(if (not (and (exact-integer? count) (>= count 0)))
(type-error "bag-decrement!" count))
(if (= count 0)
bag
(let* ((eq (comparator-equality-predicate (bag-element-comparator bag)))
(entries (bag-entries bag)))
(let loop ((prev #f) (rest entries))
(cond
((null? rest) bag)
(else
(let ((entry (car rest)))
(if (eq (bag-entry-element entry) element)
(let ((new-count (- (bag-entry-count entry) count)))
(if (> new-count 0)
(set-cdr! entry new-count)
(if prev
(set-cdr! prev (cdr rest))
(set-bag-entries! bag (cdr rest))))
bag)
(loop entry (cdr rest))))))))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

@17802723 17802723 requested a review from jetjinser January 29, 2026 08:16
Comment on lines 569 to 570
(define (bag-entry-count entry)
entry)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个没有意义,let 的时候命名清楚 entry-count 就行

Comment on lines 591 to 595
(when (> entry 0)
(let ((new-count (- (bag-entry-count entry) count)))
(if (> new-count 0)
(hash-table-set! entries element new-count)
(hash-table-delete! entries element))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

直接判断 entry > count 就行了吧

Comment on lines 54 to 55
(check-true (not (not (member 1 b-list))))
(check-true (not (not (member 2 b-list))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

两次 not 没有意义
list 直接 check 内容吧

@17802723 17802723 requested a review from jetjinser January 29, 2026 09:01
Copy link
Collaborator

@jetjinser jetjinser left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +54 to +55
(check-false (not (member 1 b-list)))
(check-false (not (member 2 b-list)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

(check-true (member 1 b-list))
(check-true (member 2 b-list))

想到确实不能直接检查 b-list 内容,因为 bag 不保证顺序,这么检查是对的。

@17802723 17802723 merged commit e6232c5 into main Jan 29, 2026
4 checks passed
@17802723 17802723 deleted the hxh/210_10/bag branch January 29, 2026 09:14
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