ueberauth / guardian

Elixir Authentication
MIT License
3.44k stars 381 forks source link

is Guardian.Plug.current_resource the best place to put it ? #416

Closed julien-leclercq closed 7 years ago

julien-leclercq commented 7 years ago

a small design question

In my mind plug should be functions that take a Plug.Conn and return a Plug.Conn so I wondered why did you decide to put current_resource/1 in Guardian.Plug.

Disclaimer : I don't expect a change since it would be breaking for most users but assuming you're more experimented developpers than I am, I wanted to ask this question :)

Best Regards

Julien

yordis commented 7 years ago

@julien-leclercq that is interesting question.

I would say that if you think from domain of the application.

Plug is a module that deals with Plug.Conn. Don't forget that this is functional programming, modules are just for encapsulation of the functions together.

By design, I would say that is all about grouping functionalities rather than OOP where you group by objects.

So Plug module will be all about Plug functionalities.

But that is my opinion.

hassox commented 7 years ago

There are many examples of functions in the Plug.Conn module that do not take and return a conn. read_body, get_session, get_req_header, send_chunked.

assign and put_private do return a conn struct but to access the data on them you access via assigns or private attributes. Guardian stores it's data in the connection in the private attribute, namespaced so as not to collide (as suggested by core members). It also provides accessors for how to get at this data. I don't believe this is an unusual pattern and tbh I'm not sure how else you'd do it.

doomspork commented 7 years ago

@julien-leclercq it sounds like the module name might be confusing? The Guardian.Plug module isn't a plug, it's a collection of helpers for working with a connection.

julien-leclercq commented 7 years ago

yep I got it now thanks for your answers mates !