Pull forward the declaration of the RouterConfig in main()

[dpw]

This eliminates a few variables, and the naming awkardness where we
have corresponding but distinct flag and router config variables.

[dpw]

Comma gone again.

(I notice that this page currently shows "rade commented on an outdated diff", confirming my suspicion that github can sometimes track PR comments across rebases.)

[rade]

The treatment of BufSz is rather suspect. It would fail a type check in a type system that supported units, i.e. the type of RouterConfig.BufSz is "bytes", yet we read a "MBytes" value into it (and fix it up later). So I reckon we should leave the intermediate var in place for that one.

Overall I find this change makes the code less regular and makes it harder to figure out what the router config contains, since it gets created bit by bit. Is that outweighed by the 11 lines saved?

[dpw]

The treatment of BufSz is rather suspect. It would fail a type check in a type system that supported units, i.e. the type of RouterConfig.BufSz is "bytes", yet we read a "MBytes" value into it (and fix it up later). So I reckon we should leave the intermediate var in place for that one.

Go is rather far from having such a type system.

Overall I find this change makes the code less regular and makes it harder to figure out what the router config contains, since it gets created bit by bit. Is that outweighed by the 11 lines saved?

The elimination of mess such as name vs ourName and password vs pwSlice is worth it on its own. And 11 lines is 11 lines.

[rade]

Go is rather far from having such a type system.

Right, but my point stands. There is a bad smell to setting a field value to something that is invalid and then mutating it to be valid.

The elimination of mess such as name vs ourName and password vs pwSlice is worth it on its own.

I don't actually consider that to be messy.

[dpw]

Go is rather far from having such a type system.

Right, but my point stands. There is a bad smell to setting a field value to something that is invalid and then mutating it to be valid.

Reintroduced with a clearer name.

[rade]

Reintroduced with a clearer name.

Cheers. As for the overall change. I am still not convinced. Perhaps ask for a 3rd opinion?

[dpw]

Perhaps ask for a 3rd opinion?

@bboreham, will you arbitrate?

[bboreham]

I like this change. Considering the way that RouterConfig gets used inside router.go, I might pull Name and NickName out and make the remaining RouterConfig a member of Router, saving another 12 lines.

[dpw]

Considering the way that RouterConfig gets used inside router.go, I might pull Name and NickName out and make the remaining RouterConfig a member of Router, saving another 12 lines.

I did think about this, and as you mention the wrinkle is Name andNickName. I anticipated resistance to moving them out of RouterConfig to separate NewRouter parameters on the basis that it would revealing implementation details.

It would be really nice if we could introduce a RouterCommon that had the common parts of Router and RouterConfig, and make it an anonymous field of both. But this uglifies the corresponding struct literals, because anonymous fields are not magical there.