I encourage approach 2a
. Approach 1. places you at risk of bad runtime and especially compile time performance, but has the advantage of extensibility without modifying the source code.
I think runtime and compile time are much more important than extensibility without modifying source. Just modify the source instead of writing other source elsewhere to extend, and avoid the runtime and compile time costs.
If you do go with some variant of 1
, I suggest at least asserting the return type.
read_file(path, ::Val{:hdf5}) = ...
read_file(path, ::Val{:binary}) = ...
read_file(path, ::Val{:text}) = ...
read_file(path, format::Symbol) = read_file(path, Val(format))::File
where Base.isconcretetype(File) == true
to make sure that no type instabilities will propagate from this dynamic dispatch.
If someone goes with 1
, that means they prioritize external extensibility. Thus, I would recommend adding
Base.Experimental.@max_methods 1 function read_file end
before your first definition, so that we always have dynamic dispatches, and thus don’t need to recompile all code calling it when someone defines a new method.
If you’re already defining enough methods to go over the default limit (3), then this isn’t strictly necessary.
The single dynamic dispatch (single, because you’ve asserted the return type to make sure type instabilities don’t propagate, so no reason for more) is fast enough that it’s fine from a performance perspective, when the function you’re calling is as slow as reading a file. It is like 20ns.