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

RFC: Use-able module #170

Open
scohen opened this issue Jan 10, 2017 · 4 comments
Open

RFC: Use-able module #170

scohen opened this issue Jan 10, 2017 · 4 comments
Labels

Comments

@scohen
Copy link
Collaborator

scohen commented Jan 10, 2017

When using elixir-thrift, I found myself doing a lot of this:

defmodule UsingThrift do  
  alias Thrift.Service.MyEnum
  alias Thrift.Service.MyOtherEnum
  require MyEnum
  require MyOtherEnum
end

I was thinking that it would be nice if we'd have something that would do the require of all the enums for us so I could do

defmodule ThriftUser do 
  use Thrift.Service
  alias Thrift.Service.{MyEnum, MyOtherEnum}
end

or

defmodule ThriftUser do 
  use Thrift.Service
end

I don't have opinions about much here, I just want to spur conversations and improve the ergonomics of our enums.

@pguillory @jparise @dantswain

@jparise jparise changed the title RFC **Use-able module*** RFC: Use-able module Jan 11, 2017
@jparise
Copy link
Collaborator

jparise commented Jan 11, 2017

It could be nice to support just use Thrift.Service. I like being explicit about the aliases, but we could perhaps extend to use statement to make that shorter:

use Thrift.Service,
  aliases: [MyEnum, MyOtherEnum]

I'm not sure that really gets us anything that the explicit aliases don't already provide though.

@scohen
Copy link
Collaborator Author

scohen commented Jan 11, 2017

I think controlling aliases has to be up to the user; and I think it should be a passthrough to the normal alias params.

I really am most concerned about eliminating the requires.

@pguillory
Copy link
Collaborator

Thrift.Service in this example is a namespace, right? As if there's a whatever.thrift like:

namespace elixir Thrift.Service
enum MyEnum {}
enum MyOtherEnum {}

We don't currently generate a module that corresponds to a namespace like that. There would be a couple of edge cases to consider if we wanted to do so. First, things can go in the root namespace, which is another discussion but that's the current behavior. Second, different IDL files can specify the same namespace, so this would introduce a kind of name collision that otherwise doesn't exist.

I don't find require to be that onerous. When writing code, the Elixir compiler tells you when you need one, then you add it, done. It's a small fixed cost. The difficulty of reading code is a much more important consideration generally, and in that respect a require costs basically nothing. You just page down past boilerplate like that. So it's hard to justify much complexity or abstraction around reducing that cost.

@jparise jparise added rfc and removed question labels Jan 12, 2017
@scohen
Copy link
Collaborator Author

scohen commented Feb 12, 2017

I don't find require to be that onerous.

@pguillory, I couldn't disagree more, in using our event thrift, I grew very tired of aliasing and requiring a half-dozen or so enums in every file.
In the above example, you'd simply do

use Thrift.Service.Enums 

And you're done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants