Question about container/heap

205 views
Skip to first unread message

Travis Keep

unread,
Jun 13, 2021, 11:09:48 AM6/13/21
to golang-nuts
Recently I implemented a heap in GO. To implement a heap, you have to implement Push() and Pop() as well as the 3 methods for sort: Less(),, Swap(), and Len().  For my heap, I never had to Push or Pop, I only had to initialize the heap and repeatedly "Fix" the top element of the heap.  As it turns out the Init and Fix functions only use the 3 sorting methods of the heap.  They don't use Push or Pop. So when I ran test coverage, I found that my Push and Pop implementations weren't covered.  I can't leave my Push and Pop implementation out because the Init and Fix methods require heaps with all 5 methods.  What do I do?

What is the preferred way in GO to implement methods of an interface that aren't really needed?  Why don't the Init and Fix methods of container/heap just accept a sort.Interface?

Brian Candler

unread,
Jun 13, 2021, 2:35:42 PM6/13/21
to golang-nuts
On Sunday, 13 June 2021 at 16:09:48 UTC+1 kee...@gmail.com wrote: 
For my heap, I never had to Push or Pop, I only had to initialize the heap and repeatedly "Fix" the top element of the heap.  As it turns out the Init and Fix functions only use the 3 sorting methods of the heap.  They don't use Push or Pop. So when I ran test coverage, I found that my Push and Pop implementations weren't covered.  I can't leave my Push and Pop implementation out because the Init and Fix methods require heaps with all 5 methods.  What do I do?

Add some unit tests (in your own application) that call heap.Push or heap.Pop on your heap type. That will give you the test coverage.

If you don't like carrying dead code, then you could replace your Push and Pop with panic() - but you'll still want unit tests that test that they panic, for your coverage report.

You could argue that heap.Init and heap.Fix could take a sort.Interface rather than a heap.Interface.  However that exposes internals, and it would limit backwards-compatible versions of those functions.

jake...@gmail.com

unread,
Jun 14, 2021, 11:44:31 AM6/14/21
to golang-nuts
In fact I would go even further and argue that it would be technically incorrect to use the Init() and Fix() methods without fully implementing the interface required by the documentation. As Brian pointed out, the fact that they are not used is an implementation detail. Unless I missed something that is not documented. As a reasonable compromise, I would definitely implement the 'missing' methods using panic(), as a hedge against future changes.  I will admit, I can not really see the implementation changing to use Push() and Pop(), but it seems like good practice and hygiene.

(I also use my car directional signals even when no one is behind me ... better to just keep in the habit of being safe.)
Reply all
Reply to author
Forward
0 new messages