Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use http.Handler instead of http.HandlerFunc #266

Closed
komuw opened this issue Jun 7, 2023 · 0 comments · Fixed by #269
Closed

use http.Handler instead of http.HandlerFunc #266

komuw opened this issue Jun 7, 2023 · 0 comments · Fixed by #269

Comments

@komuw
Copy link
Owner

komuw commented Jun 7, 2023

https://github.com/komuw/ong/blob/v0.0.48/middleware/loadshed.go#L47

https://github.com/komuw/ong/blob/v0.0.48/mux/mux.go#L34

func oneMiddleware(wrappedHandler http.HandlerFunc) http.HandlerFunc {
	count := 0
	return func(w http.ResponseWriter, r *http.Request) {
		count = count + 1
		wrappedHandler(w, r)
	}
}

func twoMiddleware(wrappedHandler http.Handler) http.HandlerFunc {
	count := 0
	return func(w http.ResponseWriter, r *http.Request) {
		count = count + 1
		wrappedHandler.ServeHTTP(w, r)
	}
}

func threeMiddleware(wrappedHandler http.Handler) http.Handler {
	count := 0
	return http.HandlerFunc(
		func(w http.ResponseWriter, r *http.Request) {
			count = count + 1
			wrappedHandler.ServeHTTP(w, r)
		},
	)
}

Ideally we want threeMiddleware (I think).
We should benchmark them.

@komuw komuw closed this as completed in #269 Jun 8, 2023
komuw added a commit that referenced this issue Jun 8, 2023
…andlerFunc (#269)

What:
- Use `http.Handler` as the http middleware instead of `http.HandlerFunc`
- With this change, a middleware accepts a `http.Handler` and returns a `http.HandlerFunc`.
  This is more in line with the mantra; `accept interfaces, return concrete types`

Why:
- `http.Handler` is a more idiomatic middleware since it is an interface whereas http.HandlerFunc is an implementation of that interface
- Fixes #266

Benchmarks:
func oneMiddleware(wrappedHandler http.HandlerFunc) http.HandlerFunc {
	count := 0
	return func(w http.ResponseWriter, r *http.Request) {
		count = count + 1
		r.Header.Set("MY-COUNT", fmt.Sprint(count))
		wrappedHandler(w, r)
	}
}

func twoMiddleware(wrappedHandler http.Handler) http.HandlerFunc {
	count := 0
	return func(w http.ResponseWriter, r *http.Request) {
		count = count + 1
		r.Header.Set("MY-COUNT", fmt.Sprint(count))
		wrappedHandler.ServeHTTP(w, r)
	}
}

func threeMiddleware(wrappedHandler http.Handler) http.Handler {
	count := 0
	return http.HandlerFunc(
		func(w http.ResponseWriter, r *http.Request) {
			count = count + 1
			r.Header.Set("MY-COUNT", fmt.Sprint(count))
			wrappedHandler.ServeHTTP(w, r)
		},
	)
}

go test -benchmem -run=^$ -bench ^BenchmarkMid$ github.com/komuw/ong/mux -count=5
BenchmarkMid/oneMiddleware-8      358.7 ns/op
BenchmarkMid/oneMiddleware-8      371.8 ns/op
BenchmarkMid/oneMiddleware-8      343.6 ns/op
BenchmarkMid/oneMiddleware-8      289.0 ns/op
BenchmarkMid/oneMiddleware-8      291.4 ns/op

BenchmarkMid/twoMiddleware-8      296.7 ns/op
BenchmarkMid/twoMiddleware-8      353.3 ns/op
BenchmarkMid/twoMiddleware-8      363.2 ns/op
BenchmarkMid/twoMiddleware-8      366.2 ns/op
BenchmarkMid/twoMiddleware-8      315.2 ns/op

BenchmarkMid/threeMiddleware-8    299.0 ns/op
BenchmarkMid/threeMiddleware-8    307.6 ns/op
BenchmarkMid/threeMiddleware-8    324.9 ns/op
BenchmarkMid/threeMiddleware-8    367.6 ns/op
BenchmarkMid/threeMiddleware-8    373.3 ns/op

The change we want is `twoMiddleware` and it seems competitive with the others.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant