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

API suggestion: replace request getter with createRequest method #27

Closed
bryanrsmith opened this issue Mar 20, 2015 · 1 comment · Fixed by #28
Closed

API suggestion: replace request getter with createRequest method #27

bryanrsmith opened this issue Mar 20, 2015 · 1 comment · Fixed by #28

Comments

@bryanrsmith
Copy link
Contributor

I think the HttpClient.request getter is an awkward way to create a new request. The getter makes it look like it's mutating the client's state, or something. I think the API would be considerably clearer if it were a method instead.

Current API:

client.request
  .withParams({ foo: 'bar' })
  .get(uri);

Proposed API:

client.createRequest()
  .withParams({ foo: 'bar' })
  .get(uri);
bryanrsmith added a commit to bryanrsmith/http-client that referenced this issue Mar 21, 2015
This change replaces the `HttpClient.request` getter with an `HttpClient.createRequest()` method to clarify the API for creating new HTTP requests.

Fixes aurelia#27
bryanrsmith added a commit to bryanrsmith/http-client that referenced this issue Mar 21, 2015
This change replaces the `HttpClient.request` getter with an `HttpClient.createRequest()` method to clarify the API for creating new HTTP requests.

This is a breaking API change. To update, replace uses of the `HttpClient.request` property with calls to the `HttpClient.createRequest()` method.

Fixes aurelia#27
@kristianmandrup
Copy link

createRequest obviously looks like a factory method, like you are building a new request instance which is more clear incase you are always creating a new instance and never reusing one. In that case you might as well aloe the params hash to be passed directly as an argument to the factory method.

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.

2 participants