probe-lab / go-kademlia

Generic Go Kademlia implementation
Other
17 stars 4 forks source link

Duplicate code in key.go #101

Closed guillaumemichel closed 10 months ago

guillaumemichel commented 10 months ago

There is a lot of duplicate code in key.go. At the first look, having a distinct type for each keysize doesn't seem like a best practice.

What was the reasoning to replace the generic key implementation with the current static one? The generic key implementation seems easier to read and more concise. I am certainly missing the benefits that the current approach brings.

iand commented 10 months ago

The duplicate code in key.go is only for convenience. go-kademilia doesn't depend on the key size but having those common ones available makes it easier to test. We could move kad8 and kad32 to kadtest and retain kad256 (perhaps other users might want to supply their own kad160)

The primary reasons for adding a generic typesafe key interface:

  1. eliminates checks for key length everywhere a key is used, such as in the routing table. Otherwise we have invalid key length errors to check for basic operations
  2. the generic types allow the caller to use their own types when working with go-kademlia
  3. it allows more efficient representations of keys rather than assuming a heap allocated byte slice, for example you could implement a Key using a struct of 4 uint64s
guillaumemichel commented 10 months ago

OK, I am now convinced :+1: