Project Structure Proposal #65
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is simply a proposal and none of this code has been manually tested
Motivation
Based on the "tools" that Go offers such as the the
internal
folder (https://www.bytesizego.com/blog/golang-internal-package), some "best practices" I have picked up while writing Go professionally and learned from long time Go developers such as Bill Kennedy (https://www.youtube.com/watch?v=bQgNYK1Z5ho) I would like to propose an updated structure for this project.I view this project not only as a great tool to increase sovereignty on the Nostr network but also as an educational opportunity for Go developers interested in contributing to the Nostr network.
The goals of this re-structuring are the following:
init.go
filesync.Mutex
for data structures that are potentially mutated inside multiple go routinesBelow are my personal thoughts/opinions and the practical actions I have taken to accomplish these goals
Remove Global Variables &
init.go
FileIn general, global variables are a code smell and should be avoided. Of course there are cases where global variables are not an issue. In the case of this codebase I found no situation where global variables were necessary therefore I was able to remove them using other patterns such as dependency injection and data encapsulation in structures.
I removed the
init.go
file and placed theinitRelays
function inmain.go
. The reasoning for this due to the removal of the global variables defined ininit.go
. Now that theinitRelays
function has all of it's dependencies passed as arguments, which are created infunc main()
, it made since to colocate this function inmain.go
.Improve Modularity & Testability
Currently the codebase has one package,
main
, and given the current stage of development, this makes sense. If this project wants to expand in scope or enable relay operators to run some of the components of this project in isolation, such as running a one-off import, modularity will need to improve. The way to improve modularity in a Go codebase is via the package system.The codebase also lacks tests which again, given it's current stage of development is completely understandable. If a goal of this project is to be software that private relay operators trust with their personal data then having code that is verifiably working is a must. As Kent Beck says, "make it work, make it right, make it fast". Currently Haven works and this is a great accomplishment, now might be a good time to start thinking about making it "right", whatever that means for this project. In my opinion, a logical step in making it "right" is to be the code testable. Making Go code testable first requires the application to be structure in a modular fashion with clear boundaries/definitions on dependencies.
Move towards this goal of modularity, I have structured the different components of this application (as I see them) into Go packages. This is done with mindset of being able to test these different components with minimal dependency on one another and when necessary, with mockable dependencies, in the future. This is how I see Go projects structured throughout the industry so I don't think I am proposing anything out-of-bounds by saying this.
For example, the
config
package has great influence on other components of this application. If a relay operator incorrectly configures their environment variables the application could attempt to do a database backup to GCP, AWS, etc. without the necessary environment variables being defined, resulting in undesired behavior or crashing the application. Ideally, theconfig
package has logic and tests that ensure that given a particular configuration, the correct application behavior will happen and in the case of invalid configuration the relay operator is able to debug what is incorrectly configured as soon as possible in the startup of the application.Prevent Data Races via
sync.Mutex
With the introduction of the
wot
package, which defines aWoT
structure to hold the state of the Web-of-Trust used in the application, I added a mutex lock to prevent potential data races when mutating the underlying state. In general, if a data structure is mutated in a go routine it is a good idea to add a mutex lock to prevent unwanted data races. While the logic of the program may not actually have any potential for data races of this state it is better to be safe than sorry.I have applied the above logic to the
importr
package as well.