I vote for C
I agree it’s a problem when libraries and apps use private functions; doing nothing may be ok for a while, but I’d rather not depend on the “There’s no documentation, therefore it’s private” convention. I enjoy the explicitness of FP and Elixir, so I think I’ll enjoy that in documentation as well with guardrails.
Here’s my issue.
In OO programming, private methods and variables make sense to restrict the manner in which the state of an object can change.
In an explicitly functional, call function, get response, no side effects context the value of preventing a method call from a space outside of the developer’s initial intent is minimal.
The case that brought this up was just from a published library with a degree of popularity utilizing something that wasn’t supposed to be utilized. A warning alone would have solved this problem because a) the developer would see it and b) anyone who installed and used the library would see it likely resulting in an issue being created, a PR being created and/or a conversation from the library developer to the team to communicate needs better.
I just don’t see any value that can come from potentially increasing the chances for errors in running code when there’s no a discernible benefit over a warning.
Somebody above said that when pulling your deps for the first time, compiling your project yields a huge amount of warnings – and the warning you talk about will be drowned in them. Don’t get me wrong: I’d mercilessly pursue any and all warnings and fix them but (a) package maintainers aren’t always responsive and (b) I have to prioritize my paid work. All of us have only so much brain and finger cycles per day.
This is only one of its goals. In any and all of the 8 programming languages I worked with, private/protected functions are also used to express the intent of “here be dragons”, namely “this here is an implementation detail, you should not ever use this by yourself, please use my public API functions”.
Additionally, in corporate teams it is also being heavily utilized to limit the possibility of unintended dependencies – f.ex. a colleague from another team scans your internal company repo, finds the perfect function to do their job and before you know it, you have 2 team leads and 10 angry developers at your door because you changed a critical API their apps can’t work without.
When this proposal gets implemented, I’ll use it to enforce boundaries in my code, be it internal corporate big mono-repos, or public open-source libraries.
This also has the added benefit of encouraging discussion of yours / somebody else’s code.
I agree with @imetallica that a private module shouldn’t be accessible at all (or maybe inside of its own file), otherwise it would be strange. Maybe a different name like protected or internal would make more sense.
But speaking about the functionality, I like the option C with some considerations. I believe that is too early to add visible_to, it adds some complexity and issues described before, and for the problem described I believe that having something like require MyApp.Private, as: Private, allow_private: true combined with defmodulep would be enough. Internally, the team should know where to use it, and, externally, you can still get a error that can be bypassed if you know what you’re doing. In this case, private maybe still makes sense, since you’re bypassing it.
I can see that and the perspective makes sense.
At the same time, I’ve been doing this professionally for over 15 years and I’ve never run into an instance where private methods were beneficial in server side code. I keep hearing the justification without seeing the benefits.
The initially described issue seems to be more of a problem solved by dependency management for version compatibility than anything else.
@josevalim although already said that elixir and the compiler doesn’t have the concept of “applications”, I think the way forward would be “internal” modules that are only accessible on the current “application”/“library”, since this would:
Which is precisely what we want to do here. I think this should be reconsidered.
I have been thinking about this approach and I even had a proof of concept and it is really inferior to the alias approach mentioned in B/C/D. For example, imagine this macro in your library:
require SomethingPrivate.Helper, as: Helper
defmacro does_something(arg) do
quote do
Helper.compute_x(unquote(arg))
end
end
If it is per project, Helper is now being used in a third party application due to meta-programming and therefore it will not work. With the require+alias system however, once you alias a private module and you have permission to use, you can pass it around and it will always work. In other words, it works more like an ownership system, which is really neat and transparent.
Perhaps, however, the worst about doing the visibility per project is that if you mess it up, like in the example above, you will only find it out when somebody else uses your project. Probably after a new version is released, and that is quite worse than any of the other proposals mentioned here, which requires you to always consider the visibility.
Finally, you have to remember that IEx is its own application and the application you are working on is lost once you build a release and connect to a remote node, which means that this approach would also make it impossible to debug your private modules in a live system.
So besides the compiler not really knowing about applications, you can see that doing something per project is more error prone than the current proposals in many ways.
I’m not a huge fan of this in general, but I’m resigned that we’re going to see it. My main point against it is the extra complexity it adds to the language that every developer is going to have to learn and deal with as long as Elixir exists. What that buys us is a reduced public perception of package upgrades being brittle. Public perception is definitely important, but at the core this is a value judgement between these two concerns.
Developers depending on undocumented modules is a “people problem.” I’ve rarely seen people problems fixed with technological solutions.
I think this is a great proposal! It’s a chance to be more explicit about which parts of codebase are the public entry points and which parts are just the private implementation details.
It’s a much more effective and intention revealing construct than @moduledoc false.
Currently, within teams/companies, developers may agree on certain conventions, such as “let’s use the public interface of module A, instead of calling A.B.C.some_function directly from D”. But it requires discipline and before you know it your code is tightly coupled and it becomes hard to reorganize or refactor those internal (private) modules.
This proposal also goes hand in hand with Phoenix Contexts: we “know” that we’re not supposed to call directly call MyApp.Blog.Post.changeset from the controller. But it is still possible and currently there is nothing in the language that prevents it. So it would be great to be able to be more explicit about it via a defmodulep, similarly to defp.
Regarding option C:
Would specifying visible_to be required? I think in almost all cases the value of visible_to: is chosen to be that of the namespace one level above. It almost seems redundant. In fact I think it’d seem odd if visible_to: would point to entirely different namespace (i.e. MyOtherApp). If I understand correctly it is not desirable/feasible to dynamically deduce visible_to automatically when not supplied.