fosrl / fosrl/newt

Go Report Card findings: reduce cyclomatic complexity and fix typo

Open
#359 0 comments 0 reactions 1 assignee Claimed by @marcschaeferger View on GitHub
Dominant language
Go
Stars
908
Forks
87
Avg merge
1d 15h
Merged PRs (30d)
7

Description

### Summary

Go Report Card currently reports no issues for `gofmt`, `go vet`, `ineffassign`, and license checks. However, there are several `gocyclo` warnings for functions with cyclomatic complexity above the recommended threshold of `15`, plus one minor spelling issue.

https://goreportcard.com/report/github.com/fosrl/newt

### Current status

#### Passing checks

- `gofmt`: 100%
- Code is formatted with `gofmt -s`
- `go vet`: 100%
- No suspicious constructs detected
- `ineffassign`: 100%
- No ineffectual assignments detected
- `license`: 100%
- LICENSE file exists

### Issues to address

#### High cyclomatic complexity

Go Report Card warns for functions with cyclomatic complexity greater than `15`.

| File | Line | Function | Complexity |
|---|---:|---|---:|
| `websocket/client.go` | 218 | `(*Client).getToken()` | 19 |
| `websocket/client.go` | 336 | `(*Client).establishConnection()` | 17 |
| `wgnetstack/wgnetstack.go` | 786 | `(*WireGuardService).calculatePeerBandwidth()` | 17 |
| `proxy/manager.go` | 290 | `(*ProxyManager).handleUDPProxy()` | 17 |
| `main.go` | 128 | `main()` | 165 |
| `main.go` | 1261 | `validateTLSConfig()` | 17 |
| `util.go` | 232 | `startPingCheck()` | 25 |
| `docker/client.go` | 159 | `ListContainers()` | 25 |
| `wg/wg.go` | 601 | `(*WireGuardService).handleUpdatePeer()` | 19 |

The most important target is `main()` in `main.go`, which currently has a complexity of `165`. This should probably be split into smaller functions for configuration loading, validation, dependency initialization, service startup, signal handling, and shutdown logic.

### Idea

- Refactor large functions into smaller, testable helper functions.
- Prioritize `main.go`, especially `main()`, because it is by far the largest complexity outlier.
- Avoid behavior changes during the refactor.
- Add or adjust tests where possible to make sure the refactoring does not change runtime behavior.
- Keep the public API and configuration behavior unchanged unless explicitly required.

### Minor typo

`misspell` reports one typo:

| File | Line | Current | Suggested |
|---|---:|---|---|
| `wg/wg.go` | 329 | `exising` | `existing` |

### Acceptance criteria

- [ ] Fix the typo in `wg/wg.go`.
- [ ] Reduce cyclomatic complexity of the listed functions where reasonably possible.
- [ ] At minimum, significantly reduce the complexity of `main()`.
- [ ] Keep existing behavior unchanged.
- [ ] `gofmt -s` passes.
- [ ] `go vet` passes.
- [ ] `ineffassign` passes.
- [ ] Go Report Card no longer reports avoidable `gocyclo` or `misspell` warnings.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.