AlchemistCamp

How would you refactor this? (shared templates helper)

I’ve just finished writing up how I share templates between Phoenix views, and it turns out the easiest and clearest way I’ve found to do it explicitly goes against the suggestion NOT to inline any functions in the my_app_web.ex file.

Basically, I’ve added this function to the view quote do part:

def render_shared(template, assigns \\ []) do
  render(MyAppWeb.SharedView, template, assigns)
end

The reason I did it because SharedView is a view and if I put the function there and import the function into my_app_web.ex, the following compile error occurs:

== Compilation error in file lib/my_app_web/views/shared_view.ex ==
** (CompileError) lib/my_app_web/views/shared_view.ex:2: you are trying to use the module MyAppWeb.SharedView which is currently being defined.

This may happen if you accidentally override the module you want to use. For example:

    defmodule MyApp do
      defmodule Supervisor do
        use Supervisor
      end
    end

In the example above, the new Supervisor conflicts with Elixir's Supervisor. This may be fixed by using the fully qualified name in the definition:

    defmodule MyApp.Supervisor do
      use Supervisor
    end

    expanding macro: MyAppWeb.__using__/1
    lib/my_app_web/views/shared_view.ex:2: MyAppWeb.SharedView (module)
    (elixir) expanding macro: Kernel.use/2
    lib/my_app_web/views/shared_view.ex:2: MyAppWeb.SharedView (module)
    (elixir) lib/kernel/parallel_compiler.ex:229: anonymous fn/4 in Kernel.ParallelCompiler.spawn_workers/7

The question is: how big of a deal is it really, to have an inline function as I currently do? And where would be a logical place to refactor the function to?

Any suggestions / feedback much appreciated!

7 comments

/phoenix #opinions

2 1084 7

2019-09-23 02:02:41 UTC

First 7 of 7 Posts!

gregvaughn

Firstly, I wouldn’t sweat a one-liner in my_app_web.ex. Things get bad when over time that grows to a dozen or two, which I think is the real caution behind the advice you heard to never do it.

However, we might could even simplify that too. What if you alias MyAppWeb.SharedView in there? Then every other view can call SharedView.render(template, assigns)

Post #1

AlchemistCamp

That’s a good idea! It’s 19 keystrokes vs 13 to use it each time, though:

SharedView.render
render_shared

Post #2

LostKobrakai

The big drawback with import is that it does cause a compile time dependency. Each time you’ll update SharedView it’ll cause recompilation on all of your view modules. Aliases on the other hand don’t do that. This was the reason for phoenix to switch from importing path helpers to aliasing the module.

Post #3

gregvaughn

Well if you want to then
alias MyAppWeb.SharedView, as: SV
and call with SV.render
but I wouldn’t recommend it. Code is read much more often than it is written.

Post #4

AlchemistCamp

Don’t want to golf and SV.render definitely isn’t as clear or searchable as either render_shared or SharedView.render. I do have a history of severe RSI, and even before that valued succinctness, though!

FWIW, I think render_shared is the least awkward name, at least how I think of what’s being done. I like the alias idea, just may need to change multiple names.

I’ll probably just stick with the current approach based on your feedback that an inline one-liner isn’t a huge problem, unless there’s a cleaner/simpler way.

Post #5

AlchemistCamp

It’s hard to imagine doing that often… the only view modules I tend to alter are those with embedded eex for meta tags and other SEO-related concerns.

Post #6

AlchemistCamp

Okay… with just one suggestion and none that maintain the same interface (which was the goal of the entire endeavor), I’ll just “accept” the current structure.

Post #7