How best to clean up this warning

[Brian Cardarella]

In one of my libraries this line throws a warning:

https://github.com/DockYard/inquisitor/blob/master/test/inquisitor_test.exs#L54

"unused import Ecto.Query"

because the `use` module above will @before_compile the following:

https://github.com/DockYard/inquisitor/blob/master/lib/inquisitor.ex#L83

so technically the Whitelist module has calls to Ecto.Query but for whatever reason Elixir doesn't think so.

Yes, this is a test case but I'm curious if there are ways to clean this up?

[Michał Muskała]

To be honest, I have a really hard time to understand why this library is using macros at all.

All of this can be achieved with just functions, in a much clearer way (although that’s obviously just my opinion).

Using functions also allows you to have multiple different builders, should you need them, and completely decouples the query from the module.

I’ve created a simple gist to showcase a possible, macro-less, solution: https://gist.github.com/michalmuskala/ade699c25caccb51760a4c0c050b5707

Michał.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-ta...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/aa3b0d82-9a47-4296-ba64-47ca5f25271c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Brian Cardarella]

The use case for macros is for model-specific functions. You don't want the same matched functions applying to all models

[Michał Muskała]

If I want a different encoding I could just pass a different builder function. There’s nothing stopping you from doing:

MyApp.Post

|> Inquisitor.build_query(params, builder: &post_builder/2)

MyApp.Comment

|> Inquisitor.build_query(params, builder: &comment_builder/2)

But since those are just functions, you can hide them behind another one:

def post_query(params) do

  MyApp.Post

  |> Inquisitor.build_query(params, builder: &post_builder/2)

end

This effectively gives you the same interface as the macro approach, but is explicit and without any magic at any point.

Even better I can define two different functions for searching posts for different actions:

def current_post_query(params) do

  MyApp.Post

  |> Inquisitor.build_query(params, builder: &post_current_builder/2)

end

This is not possible with the macro approach, that implicitly couples model name to the function name behind the scenes.

I’m sorry, if I appear rude. This is definitely not my intention. I only wanted to show that functions are enough in many, many cases and macros

just obscure things. I am big believer in making libraries use functions first and foremost and later only sprinkle some macros where (and if)

necessary to hide annoying boilerplate.

Michał.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/911d9bfc-f93a-4fa3-a96b-337235a2e83e%40googlegroups.com.

[Brian Cardarella]

I'll take it into consideration.

I'm still curious about the original question. Assuming refactoring away from macros is not an option, is there a way to eliminate the warning?

[Michał Muskała]

Oh, sorry.

Import will complain when you don’t call the “bare” imported functions (without module name). Calling those same functions fully qualified (with module name) does not count as using the import.

Michał.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/5729f878-b72b-4528-8dd4-6a92d3bc9b39%40googlegroups.com.

[Eric Meadows-Jönsson]

You can disable import warnings with: import Foo, warn: false.

​

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/8B8149B6-59C1-4152-9CF4-60DC9EDD9664%40muskala.eu.

For more options, visit https://groups.google.com/d/optout.

--

Eric Meadows-Jönsson

[José Valim]

You can disable import warnings with: import Foo, warn: false.

Yes, but don't do that. :)

You should not "import" Ecto.Query but rather "require" it, since you just want to use its macros and not import them into the current scope.

[Brian Cardarella]

Thanks Jose!