Closed benpate closed 9 years ago
Since it touches the same part of the code, this branch is waiting behind PR #28. I'll make a PR for this branch as soon as #28 is "OK"
I actually don't necessarily agree with this issue. I don't really think that it's hard at all to create a Wrecker struct -- and it really only has to be done once.
While yes, this done provide some convenience, but it also creates some ambiguity and bulks up the interface some more by providing more than one way to use the library.
Let's look at this scenario.
wrecker := &wrecker.Wrecker{
BaseURL: "https://www.google.com",
HttpClient: &http.Client{
Timeout: 10 * time.Second,
},
}
wrecker.Get("/") // Same syntax as the proposed new function, but actually calls on the struct
So while it's really bad practice to name variables the same name as their packages, we run into this situation where it may actually not be entirely sure what will get called without understanding the language.
So that one situation isn't the main reason why I'm against this, but it's more of me disagreeing with allowing multiple ways to interact with this library. I would prefer to have a single standardized way of using the library + forcing the users to create a Wrecker instance.
No worries. I agree that it could cause a little confusion. I thought I’d throw that out there and see if the shortcut made sense to you. I’ll remove the pull request. :)
— Ben
On Jul 2, 2015, at 3:11 PM, Brandon Romano notifications@github.com wrote:
I actually don't necessarily agree with this issue. I don't really think that it's hard at all to create a Wrecker struct -- and it really only has to be done once.
While yes, this done provide some convenience, but it also creates some ambiguity and bulks up the interface some more by providing more than one way to use the library.
Let's look at this scenario.
wrecker := &wrecker.Wrecker{ BaseURL: "https://www.google.com", HttpClient: &http.Client{ Timeout: 10 * time.Second, }, }
wrecker.Get("/") // Same syntax as the proposed new function, but actually calls on the struct So while it's really bad practice to name variables the same name as their packages, we run into this situation where it may actually not be entirely sure what will get called without understanding the language.
So that one situation isn't the main reason why I'm against this, but it's more of me disagreeing with allowing multiple ways to interact with this library. I would prefer to have a single standardized way of using the library + forcing the users to create a Wrecker instance.
— Reply to this email directly or view it on GitHub https://github.com/BrandonRomano/wrecker/issues/26#issuecomment-118169670.
How do you feel about some convenience functions like:
They would create a new wrecker instance and initalize a Get request all in one step.