Why does calling the Pop method on a container/heap queue directly return the wrong item?

## Two functions, one name The single most common bug against `container/heap` is calling the method where you meant the package function. The names collide deliberately — the package function is *implemented in terms of* your method — but they do very different things. ### What your Pop method is for The interface documents `Pop() any` as "remove and return element `Len() - 1`". It is the storage primitive. A slice-backed implementation looks like this and contains no comparison at all: ``` func (q *JobQueue) Pop() any { old := *q n := len(old) job := old[n-1] old[n-1] = nil *q = old[:n-1] return job } ``` Nothing here knows which job is urgent. If you call `q.Pop()` from your scheduler, you get the element that happens to be last in the array — for a heap that is a **leaf**, typically one of the *worst*-ranked elements, and certainly not the root. ### What heap.Pop does `heap.Pop(h)` performs three steps: 1. `h.Swap(0, h.Len()-1)` — move the root, the element you actually want, to the end of the slice. 2. Sift the element now at index 0 downward, swapping it with its smaller child until the invariant `!Less(child, parent)` holds again. 3. `h.Pop()` — call *your* method to detach the element parked at the end. So your method is the last line of the algorithm, not the algorithm. The return value is `any`, so you type-assert: `job := heap.Pop(&q).(*Job)`. ### The same split on the way in `Push` mirrors it. Your `Push(x any)` appends `x` at index `Len()`. `heap.Push(h, x)` calls your method and then sifts the new last element **up** toward the root until it is no longer `Less` than its parent. Call `q.Push(job)` directly and the job lands as a leaf in an arbitrary spot; the array is still a valid slice but no longer a valid heap, and every later `heap.Pop` may hand back a non-minimum. Nothing panics. Nothing logs. The queue simply starts lying. ### The third way in: building the slice yourself A scheduler that loads pending jobs from a database usually builds the slice in a loop: ``` for rows.Next() { q = append(q, scanJob(rows)) } ``` That slice is in database order, not heap order. `heap.Init(&q)` must run before the first `heap.Pop`, and it is O(n) — cheaper than n separate `heap.Push` calls. Skipping it is the same class of bug: the invariant was never established, so the package's sifting has nothing correct to preserve. ### How to stop making the mistake The durable fix is encapsulation. Do not export the queue type, and do not let the rest of the daemon touch it. Wrap it: ``` func (s *Scheduler) Add(j *Job) { heap.Push(&s.queue, j) } func (s *Scheduler) Next() *Job { return heap.Pop(&s.queue).(*Job) } func (s *Scheduler) Peek() *Job { return s.queue[0] } ``` Now the only calls to `heap.*` live in three one-line methods that a reviewer can check once. Note `Peek`: reading `q[0]` without removing it is legal and cheap, because the root *is* the minimum — that is the one direct slice access that is always safe. ### Why the nil-out in Pop matters `*q = old[:n-1]` only shrinks the length. The pointer to the popped `*Job` is still sitting in the backing array's spare capacity, so the garbage collector cannot free that job for as long as the queue's array lives. In a long-running daemon that drains and refills the same queue, that is a slow leak of exactly the objects you thought you were done with. Assigning `old[n-1] = nil` before reslicing releases it. The standard library's own priority-queue example does this, and it is the sort of detail an interviewer uses to tell someone who has read the package from someone who has used it. ### The tell in review When you see `q.Pop()` or `q.Push(x)` anywhere outside the heap methods themselves, it is almost always a bug. `go vet` will not catch it — both calls are perfectly well-typed.