Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

refactor AddNode* methods in swarm/network/simulation #1059

Closed
acud opened this issue Dec 14, 2018 · 1 comment
Closed

refactor AddNode* methods in swarm/network/simulation #1059

acud opened this issue Dec 14, 2018 · 1 comment
Labels
cleanup code completion, add comments and more infrastructure simulations

Comments

@acud
Copy link
Member

acud commented Dec 14, 2018

as discussed in #1057, our methods of adding nodes into a simulation are spaghetti and have to be cleaned up and refactored, with the appropriate functionalities moved to p2p/simulations package

@acud acud added simulations infrastructure cleanup code completion, add comments and more labels Dec 14, 2018
@frncmx frncmx self-assigned this Dec 18, 2018
@frncmx
Copy link
Contributor

frncmx commented Jan 11, 2019

The original idea was to move node.go from swarm/network/simulation to p2p/simulations. The methods in node.go currently belong to type Simulation, those methods would belong to type Network after the refactor. However, Network has too many methods already and we would double their count (see picture). That we don't want.

As I noticed, most of the methods in node.go and connect.go are not in use. I considered those as dead code and tried to remove in #1076. However the removal was voted down, as we consider the methods as part of a test helper library.

Another idea from @justelad was to simplify the interface to something like:

Connect(ids []enode.ID,  topology Topology)
AddNodes(count int, topology Topology, opts ...AddNodeOption)

But we agreed with @zelig; we don't want to spend that effort on this right now.

image

@frncmx frncmx removed their assignment Jan 11, 2019
@acud acud closed this as completed May 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cleanup code completion, add comments and more infrastructure simulations
Projects
None yet
Development

No branches or pull requests

2 participants