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

Unnecessary engine.With(opts...) call #3967

Open
raashidanwar opened this issue May 15, 2024 · 5 comments
Open

Unnecessary engine.With(opts...) call #3967

raashidanwar opened this issue May 15, 2024 · 5 comments

Comments

@raashidanwar
Copy link

Description

I was looking at the code and saw that in the Default method engine := New() where New method is returning engine.With(opts...) and again the Default method also returning engine.With(opts...) which I think is redundant engine.With call 🤔.

Expectations

engine.With(opts...) is sufficient and Default should just return engine.

@flc1125
Copy link
Contributor

flc1125 commented May 15, 2024

Because Default also needs to support opts, and in Default, some logic should prioritize opts over the defaults (such as middleware).

@raashidanwar
Copy link
Author

If we pass the opt to New, will it do the job right?

@flc1125
Copy link
Contributor

flc1125 commented May 15, 2024

Certainly. There are many ways to use it:

package main

import "github.com/gin-gonic/gin"

func main() {
	router:=gin.New(
		routes,
		route(),
		middlewares,
	)

	router.
		With(middlewares).
		GET("/v2", func(c *gin.Context) {
			c.JSON(200, gin.H{"message": "hello, world"})
		})

	router.Run(":8080")
}

func routes(e *gin.Engine) {
	e.GET("/", func(c *gin.Context) {
		c.JSON(200, gin.H{"message": "hello, world"})
	})
}

func route() func(*gin.Engine) {
	return func(e *gin.Engine) {
		e.GET("/v1", func(c *gin.Context) {
			c.JSON(200, gin.H{"message": "hello, world"})
		})
	}
}

func middlewares(e *gin.Engine) {
	e.Use(gin.Logger())
	e.Use(gin.Recovery())
}

@flc1125
Copy link
Contributor

flc1125 commented May 16, 2024

If there are no further issues, you might consider closing the issue. Thank you very much. 😁

@dukepan2005
Copy link

Sorry, I agree with @raashidanwar. In my humble opinion, I didn't see any advantage of "With" function. It seems we can add router and middleware in good ways before.

There are many articles to explain the benefit of "the functional Options pattern", Sorry, but I didn't see it there. It seems make gin further away from simplicity.

@flc1125 Can you introduce more scene where the traditional way is clumsy?

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

No branches or pull requests

3 participants