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

Renaming ROM::CommandRegistry#as to #map_to #235

Closed
c0 opened this issue May 21, 2015 · 13 comments
Closed

Renaming ROM::CommandRegistry#as to #map_to #235

c0 opened this issue May 21, 2015 · 13 comments

Comments

@c0
Copy link
Contributor

c0 commented May 21, 2015

One of the things that's confusing as a new ROM user is that #as is used with a mapper. From the TODO example —

class TaskMapper < ROM::Mapper
  relation :tasks
  model Task
end

class TasksRelation < ROM::Relation[:sql]
end

When I run —

rom.relation(:tasks).as(:tasks)

It's not clear what the second :tasks is or where it comes from.

I would like to recommend it be renamed to be more descriptive like map_to. Open to other ideas.

rom.relation(:tasks).map_to(:tasks)
@nepalez
Copy link
Member

nepalez commented May 21, 2015

@co, actually the mapper can map data without any model at all -- to pure hash. That's why naming it after the model is not a good idea. Instead, you can explicitly define the name of the mapper:

class TaskMapper < ROM::Mapper
  relation :tasks
  register_as :mapper_name
  model Task
end

then

rom.relation(:tasks).as(:mapper_name)

@c0
Copy link
Contributor Author

c0 commented May 21, 2015

@nepalez I don't think the confusion is in the naming of the mapper, but that #as does not have any indication that it's using a mapper.

rom.relation(:tasks).as(:tasks)

# as opposed to
rom.relation(:tasks).map_to(:tasks)

# or even
rom.relation(:tasks).mapper(:tasks)

@nepalez
Copy link
Member

nepalez commented May 21, 2015

I see. As for me, I feel conformed with as, but maybe you're right.
I think the substantial part of the confusion is a lack of documentation. Feel it as a stimulus for myself to write it faster :)

@c0
Copy link
Contributor Author

c0 commented May 21, 2015

I agree that documentation is immensely useful, but it's better when the code is clear without it. Both are needed 😄

@solnic
Copy link
Member

solnic commented May 21, 2015

as is an alias of map_with and we still haven’t decided which one is
better :)

On Thu, May 21, 2015 at 4:32 PM Caleb Wright notifications@github.com
wrote:

I agree that documentation is immensely useful, but it's better when the
code is clear without it. Both are needed :)


Reply to this email directly or view it on GitHub
#235 (comment).

@c0
Copy link
Contributor Author

c0 commented May 21, 2015

@solnic good to know. I personally like #map_with

@solnic
Copy link
Member

solnic commented May 21, 2015

btw you can pass multiple mapper names there ie rom.relation(:users).map_with(:entity, :view_presenter) which for me is an awesome feature :D

@cflipse
Copy link
Collaborator

cflipse commented May 21, 2015

eh, I actually prefer as ... I want to get the results of this relation as a :user_with_tasks ... makes plenty of sense.

@solnic
Copy link
Member

solnic commented May 21, 2015

me too, it reads nice imo just like you said. I guess we need to make a decision and stick to just one for 1.0.0 :)

@maetl
Copy link
Contributor

maetl commented May 22, 2015

I prefer as too. Having map_with there as an alias is okay from my perspective. It’s potentially a more intricate API than it needs to be, but I can see uses of both for readability reasons, depending on the particular style of client code.

For example, Sequel aliases where and filter.

@c0
Copy link
Contributor Author

c0 commented May 22, 2015

The way I read as is that it's expecting a noun. i.e. the object/model you'll end up with. I see mappers more as verbs and transformative rather than an end result. So something like rom.relation(:users).through(:entity, :view_presenter) makes more sense.

At the end of the day, I'll follow the consensus. I've been a ruby dev for a number of years, but am new to rom-rb and just wanted to surface some things that may be difficult for newcomers :)

@solnic
Copy link
Member

solnic commented May 23, 2015

Let's see what people prefer and decide for 1.0.0 how it should be named. Closing this for now.

@solnic solnic closed this as completed May 23, 2015
@c0
Copy link
Contributor Author

c0 commented May 23, 2015

Sounds like a plan. Thanks everyone for the feedback!

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

5 participants