probe-lab / go-kademlia

Generic Go Kademlia implementation
Other
17 stars 4 forks source link

Feedback from @marcopolo #36

Open guillaumemichel opened 1 year ago

guillaumemichel commented 1 year ago

Feedback from @marcopolo

I watched your walkthrough last night. Great stuff!

I love the trace view of the system

one yellow flag for me was the the modularization/abstraction of everything. I would push against having an interface for a single concrete implementation. Instead try keeping things concrete until you need multiple concrete implementations.

You can still control what users of the concrete implementation do by controlling the public methods.

You know a lot more about what makes sense for the interface once you have used it a bit more, and the moment you need to split things to support multiple concrete implementations is the point when you add the interfaces.

guillaumemichel commented 1 year ago

There are already multiple implementations for many modules. But I agree, for instance in the scheduling logic, all modules have a single implementation, so it probably makes sense to remove the interfaces there, and have a single scheduler implementation.

However, we want to have multiple message formats, message endpoints, routing tables, and message handlers (server)

MarcoPolo commented 1 year ago

However, we want to have multiple message formats, message endpoints, routing tables, and message handlers (server)

I’m not suggesting we don’t have multiple concrete implementations, just that before we do we stick to a single concrete implementation. This doesn’t apply to cases where we already have multiple concrete implementations.