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

Proposal: Req.get_header(req_or_resp) and Req.put_header #207

Closed
wojtekmach opened this issue Jul 26, 2023 · 6 comments
Closed

Proposal: Req.get_header(req_or_resp) and Req.put_header #207

wojtekmach opened this issue Jul 26, 2023 · 6 comments
Assignees

Comments

@wojtekmach
Copy link
Owner

wojtekmach commented Jul 26, 2023

For the vast majority of cases people should be fine just using the Req module (and looking up doctests in Req.Steps) so having Req.get_header and Req.put_header (as opposed to Req.Request.* and Req.Response.*) would go towards that.

Thanks @zachallaun for the suggestion.

Examples:

iex> Req.get_header(req, "accept")
["application/json"]

iex> Req.get_header(resp, "content-type")
["application/json"]
@wojtekmach
Copy link
Owner Author

Instead of Req.put_header, can add Req.put_headers.

But then we have two very similar ways:

Req.update(req, headers: [accept: "application/json"])

Req.put_headers(req, accept: "application/json")

so unsure.

@yordis
Copy link

yordis commented Jul 31, 2023

For the given use case of headers, I would recommend using put_headers rather than update.

The benefit of using put_headers is that it allows for easier pipelining and avoids the risk of misspelling headers in the map type. Additionally, the ergonomic design is a secondary advantage.

@wojtekmach
Copy link
Owner Author

thanks for feedback!
Misspellings are not an issue because we fail on unknown options. Can you elaborate on:

Additionally, the ergonomic design is a secondary advantage.

@yordis
Copy link

yordis commented Aug 1, 2023

Misspellings are not an issue because we fail on unknown options.

Right, which update could also fall back to put_*.

Additionally, the ergonomic design is a secondary advantage.

Staying into a put_ that already carries on the "intent" of a given permutation.

So when you are coding, you can focus on creating functions around the data and have less to do with the permutation + data (in other words, they are separated).

For example, people could end up doing the following (nuances, please understand that it depends, just based on my experience):

defp put_default_headers(reg, headers) do
  Req.update(req, [])
end

Chasing the "intent" of avoiding mistakes in terms of what "putting" means and what "putting headers" means.

Instead of:

defp default_headers do
  []
end

# or even # defp get_default_headers do
# or much closer @default_headers []

This is a simple situation, but overall, the more you separate the permutation from the data, the "easier" (🧂) it becomes to test and combine things (take it with a grain of salt 🧂, again, it depends).

Something like that,

I found that programmers will end up, in one way or another adding such specialized functions to hide away internal data structure details or avoid those naming situations.

🚲 Bikeshedding 🐑

@zachallaun
Copy link
Contributor

I personally like Req.get_header and Req.put_headers.

I'm not overly concerned about the overlap with Req.update. From a findability and UX perspective, it's more likely that a meaningful name like put_headers will be more obvious vs. a generic name like update. The docs for put_headers could include a note:

To update other fields and options at the same time as headers, see update/2.

@wojtekmach wojtekmach self-assigned this Aug 9, 2023
@wojtekmach wojtekmach changed the title Consider Req.get_header(req_or_resp) and Req.put_header Proposal: Req.get_header(req_or_resp) and Req.put_header Aug 9, 2023
@wojtekmach
Copy link
Owner Author

Closing in favour of #224.

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